Skip to content

Commit 94dfb6c

Browse files
committed
feat(sacp)!: merge pending_tasks into JrResponder and remove 'scope lifetime
- Add Role parameter to JrResponder<Role> trait - Change run() method to take JrConnectionCx<Role> at runtime - Create SpawnedResponder to replace PendingTask mechanism - Remove 'scope lifetime from JrConnectionBuilder and JrConnection - Update with_spawned to use with_responder internally - Simplify Task to always be 'static (no lifetime parameter) - Box futures in ChainResponder::run to prevent stack overflow BREAKING CHANGE: JrResponder now requires a Role type parameter and run() takes JrConnectionCx<Role>. The 'scope lifetime has been removed from JrConnectionBuilder and JrConnection.
1 parent 3d7202e commit 94dfb6c

14 files changed

Lines changed: 134 additions & 197 deletions

File tree

src/sacp-conductor/src/conductor.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,10 @@ impl Conductor {
191191
self
192192
}
193193

194-
pub fn into_connection_builder(self) -> JrConnectionBuilder<'static, ConductorMessageHandler> {
194+
pub fn into_connection_builder(
195+
self,
196+
) -> JrConnectionBuilder<ConductorMessageHandler, impl sacp::JrResponder<ConductorToClient>>
197+
{
195198
let (mut conductor_tx, mut conductor_rx) = mpsc::channel(128 /* chosen arbitrarily */);
196199

197200
let mut state = ConductorHandlerState {

src/sacp-conductor/tests/scoped_mcp_server.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ struct ScopedProxy;
8585

8686
fn make_mcp_server<Role: JrRole>(
8787
values: &Mutex<Vec<String>>,
88-
) -> McpServer<Role, impl sacp::JrResponder + use<'_, Role>>
88+
) -> McpServer<Role, impl sacp::JrResponder<Role> + use<'_, Role>>
8989
where
9090
Role: HasEndpoint<Agent>,
9191
{

src/sacp-conductor/tests/test_session_id_in_mcp_tools.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,11 @@ fn create_echo_proxy() -> Result<sacp::DynComponent, sacp::Error> {
4747
Ok(sacp::DynComponent::new(ProxyWithEchoServer { mcp_server }))
4848
}
4949

50-
struct ProxyWithEchoServer<R: sacp::JrResponder> {
50+
struct ProxyWithEchoServer<R: sacp::JrResponder<ProxyToConductor>> {
5151
mcp_server: McpServer<ProxyToConductor, R>,
5252
}
5353

54-
impl<R: sacp::JrResponder + 'static> Component for ProxyWithEchoServer<R> {
54+
impl<R: sacp::JrResponder<ProxyToConductor> + 'static> Component for ProxyWithEchoServer<R> {
5555
async fn serve(self, client: impl Component) -> Result<(), sacp::Error> {
5656
ProxyToConductor::builder()
5757
.name("echo-proxy")

src/sacp-test/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ pub fn process(data: &str) -> Result<String, crate::Error> {
247247
}
248248

249249
// Helper to create a mock connection for examples
250-
pub fn mock_connection() -> JrConnectionBuilder<'static, NullHandler<ClientToAgent>> {
250+
pub fn mock_connection() -> JrConnectionBuilder<NullHandler<ClientToAgent>> {
251251
ClientToAgent::builder()
252252
}
253253

src/sacp/src/jsonrpc.rs

Lines changed: 38 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,9 @@ use crate::jsonrpc::dynamic_handler::DynamicHandlerMessage;
3030
pub use crate::jsonrpc::handlers::NullHandler;
3131
use crate::jsonrpc::handlers::{MessageHandler, NotificationHandler, RequestHandler};
3232
use crate::jsonrpc::outgoing_actor::{OutgoingMessageTx, send_raw_message};
33+
use crate::jsonrpc::responder::SpawnedResponder;
3334
use crate::jsonrpc::responder::{ChainResponder, JrResponder, NullResponder};
34-
use crate::jsonrpc::task_actor::{PendingTask, Task, TaskTx};
35+
use crate::jsonrpc::task_actor::{Task, TaskTx};
3536
use crate::mcp_server::{McpMessageHandler, McpServer};
3637
use crate::role::{HasDefaultEndpoint, HasEndpoint, JrEndpoint, JrRole};
3738
use crate::{Agent, Client, Component};
@@ -533,20 +534,17 @@ impl<H: JrMessageHandlerSend> JrMessageHandler for H {
533534
/// # }
534535
/// ```
535536
#[must_use]
536-
pub struct JrConnectionBuilder<'scope, H: JrMessageHandler, R: JrResponder = NullResponder> {
537+
pub struct JrConnectionBuilder<H: JrMessageHandler, R: JrResponder<H::Role> = NullResponder> {
537538
name: Option<String>,
538539

539540
/// Handler for incoming messages.
540541
handler: H,
541542

542543
/// Responder for background tasks.
543544
responder: R,
544-
545-
/// Pending tasks
546-
pending_tasks: Vec<PendingTask<'scope, H::Role>>,
547545
}
548546

549-
impl<Role: JrRole> JrConnectionBuilder<'_, NullHandler<Role>, NullResponder> {
547+
impl<Role: JrRole> JrConnectionBuilder<NullHandler<Role>, NullResponder> {
550548
/// Create a new JrConnection with the given role.
551549
/// This type follows a builder pattern; use other methods to configure and then invoke
552550
/// [`Self::serve`] (to use as a server) or [`Self::with_client`] to use as a client.
@@ -555,24 +553,22 @@ impl<Role: JrRole> JrConnectionBuilder<'_, NullHandler<Role>, NullResponder> {
555553
name: Default::default(),
556554
handler: NullHandler::new(role),
557555
responder: NullResponder,
558-
pending_tasks: Default::default(),
559556
}
560557
}
561558
}
562559

563-
impl<H: JrMessageHandler> JrConnectionBuilder<'_, H, NullResponder> {
560+
impl<H: JrMessageHandler> JrConnectionBuilder<H, NullResponder> {
564561
/// Create a new connection builder with the given handler.
565562
pub fn new_with(handler: H) -> Self {
566563
Self {
567564
name: Default::default(),
568565
handler,
569566
responder: NullResponder,
570-
pending_tasks: Default::default(),
571567
}
572568
}
573569
}
574570

575-
impl<'scope, H: JrMessageHandler, R: JrResponder> JrConnectionBuilder<'scope, H, R> {
571+
impl<H: JrMessageHandler, R: JrResponder<H::Role>> JrConnectionBuilder<H, R> {
576572
/// Set the "name" of this connection -- used only for debugging logs.
577573
pub fn name(mut self, name: impl ToString) -> Self {
578574
self.name = Some(name.to_string());
@@ -584,78 +580,62 @@ impl<'scope, H: JrMessageHandler, R: JrResponder> JrConnectionBuilder<'scope, H,
584580
/// Prefer [`Self::on_receive_request`] or [`Self::on_receive_notification`].
585581
/// This is a low-level method that is not intended for general use.
586582
pub fn with_connection_builder<H1, R1>(
587-
mut self,
588-
other: JrConnectionBuilder<'scope, H1, R1>,
589-
) -> JrConnectionBuilder<'scope, ChainedHandler<H, NamedHandler<H1>>, ChainResponder<R, R1>>
583+
self,
584+
other: JrConnectionBuilder<H1, R1>,
585+
) -> JrConnectionBuilder<ChainedHandler<H, NamedHandler<H1>>, ChainResponder<R, R1>>
590586
where
591587
H1: JrMessageHandler<Role = H::Role>,
592-
R1: JrResponder,
588+
R1: JrResponder<H::Role>,
593589
{
594-
self.pending_tasks.extend(
595-
other
596-
.pending_tasks
597-
.into_iter()
598-
.map(|t| t.named(other.name.clone())),
599-
);
600-
601590
JrConnectionBuilder {
602591
name: self.name,
603592
handler: ChainedHandler::new(
604593
self.handler,
605594
NamedHandler::new(other.name, other.handler),
606595
),
607596
responder: ChainResponder::new(self.responder, other.responder),
608-
pending_tasks: self.pending_tasks,
609597
}
610598
}
611599

612600
/// Add a new [`JrMessageHandler`] to the chain.
613601
///
614602
/// Prefer [`Self::on_receive_request`] or [`Self::on_receive_notification`].
615603
/// This is a low-level method that is not intended for general use.
616-
pub fn with_handler<H1>(
617-
self,
618-
handler: H1,
619-
) -> JrConnectionBuilder<'scope, ChainedHandler<H, H1>, R>
604+
pub fn with_handler<H1>(self, handler: H1) -> JrConnectionBuilder<ChainedHandler<H, H1>, R>
620605
where
621606
H1: JrMessageHandler<Role = H::Role>,
622607
{
623608
JrConnectionBuilder {
624609
name: self.name,
625610
handler: ChainedHandler::new(self.handler, handler),
626611
responder: self.responder,
627-
pending_tasks: self.pending_tasks,
628612
}
629613
}
630614

631615
/// Add a new [`JrResponder`] to the chain.
632-
pub fn with_responder<R1>(
633-
self,
634-
responder: R1,
635-
) -> JrConnectionBuilder<'scope, H, ChainResponder<R, R1>>
616+
pub fn with_responder<R1>(self, responder: R1) -> JrConnectionBuilder<H, ChainResponder<R, R1>>
636617
where
637-
R1: JrResponder,
618+
R1: JrResponder<H::Role>,
638619
{
639620
JrConnectionBuilder {
640621
name: self.name,
641622
handler: self.handler,
642623
responder: ChainResponder::new(self.responder, responder),
643-
pending_tasks: self.pending_tasks,
644624
}
645625
}
646626

647627
/// Enqueue a task to run once the connection is actively serving traffic.
648628
#[track_caller]
649-
pub fn with_spawned<F>(
650-
mut self,
651-
task: impl FnOnce(JrConnectionCx<H::Role>) -> F + Send + 'scope,
652-
) -> Self
629+
pub fn with_spawned<F, Fut>(
630+
self,
631+
task: F,
632+
) -> JrConnectionBuilder<H, ChainResponder<R, SpawnedResponder<F>>>
653633
where
654-
F: Future<Output = Result<(), crate::Error>> + Send + 'scope,
634+
F: FnOnce(JrConnectionCx<H::Role>) -> Fut + Send,
635+
Fut: Future<Output = Result<(), crate::Error>> + Send,
655636
{
656637
let location = Location::caller();
657-
self.pending_tasks.push(PendingTask::new(location, task));
658-
self
638+
self.with_responder(SpawnedResponder::new(location, task))
659639
}
660640

661641
/// Register a handler for messages that can be either requests OR notifications.
@@ -698,7 +678,6 @@ impl<'scope, H: JrMessageHandler, R: JrResponder> JrConnectionBuilder<'scope, H,
698678
self,
699679
op: F,
700680
) -> JrConnectionBuilder<
701-
'scope,
702681
ChainedHandler<
703682
H,
704683
MessageHandler<H::Role, <H::Role as JrRole>::HandlerEndpoint, Req, Notif, F>,
@@ -762,7 +741,6 @@ impl<'scope, H: JrMessageHandler, R: JrResponder> JrConnectionBuilder<'scope, H,
762741
self,
763742
op: F,
764743
) -> JrConnectionBuilder<
765-
'scope,
766744
ChainedHandler<H, RequestHandler<H::Role, <H::Role as JrRole>::HandlerEndpoint, Req, F>>,
767745
R,
768746
>
@@ -824,7 +802,6 @@ impl<'scope, H: JrMessageHandler, R: JrResponder> JrConnectionBuilder<'scope, H,
824802
self,
825803
op: F,
826804
) -> JrConnectionBuilder<
827-
'scope,
828805
ChainedHandler<
829806
H,
830807
NotificationHandler<H::Role, <H::Role as JrRole>::HandlerEndpoint, Notif, F>,
@@ -858,11 +835,7 @@ impl<'scope, H: JrMessageHandler, R: JrResponder> JrConnectionBuilder<'scope, H,
858835
self,
859836
endpoint: End,
860837
op: F,
861-
) -> JrConnectionBuilder<
862-
'scope,
863-
ChainedHandler<H, MessageHandler<H::Role, End, Req, Notif, F>>,
864-
R,
865-
>
838+
) -> JrConnectionBuilder<ChainedHandler<H, MessageHandler<H::Role, End, Req, Notif, F>>, R>
866839
where
867840
H::Role: HasEndpoint<End>,
868841
F: AsyncFnMut(MessageCx<Req, Notif>, JrConnectionCx<H::Role>) -> Result<T, crate::Error>
@@ -898,7 +871,7 @@ impl<'scope, H: JrMessageHandler, R: JrResponder> JrConnectionBuilder<'scope, H,
898871
self,
899872
endpoint: End,
900873
op: F,
901-
) -> JrConnectionBuilder<'scope, ChainedHandler<H, RequestHandler<H::Role, End, Req, F>>, R>
874+
) -> JrConnectionBuilder<ChainedHandler<H, RequestHandler<H::Role, End, Req, F>>, R>
902875
where
903876
H::Role: HasEndpoint<End>,
904877
F: AsyncFnMut(
@@ -925,11 +898,7 @@ impl<'scope, H: JrMessageHandler, R: JrResponder> JrConnectionBuilder<'scope, H,
925898
self,
926899
endpoint: End,
927900
op: F,
928-
) -> JrConnectionBuilder<
929-
'scope,
930-
ChainedHandler<H, NotificationHandler<H::Role, End, Notif, F>>,
931-
R,
932-
>
901+
) -> JrConnectionBuilder<ChainedHandler<H, NotificationHandler<H::Role, End, Notif, F>>, R>
933902
where
934903
H::Role: HasEndpoint<End>,
935904
F: AsyncFnMut(Notif, JrConnectionCx<H::Role>) -> Result<T, crate::Error> + Send,
@@ -955,14 +924,10 @@ impl<'scope, H: JrMessageHandler, R: JrResponder> JrConnectionBuilder<'scope, H,
955924
/// .serve(connection)
956925
/// .await?;
957926
/// ```
958-
pub fn with_mcp_server<Role: JrRole, McpR: JrResponder>(
927+
pub fn with_mcp_server<Role: JrRole, McpR: JrResponder<Role>>(
959928
self,
960929
server: McpServer<Role, McpR>,
961-
) -> JrConnectionBuilder<
962-
'scope,
963-
ChainedHandler<H, McpMessageHandler<Role>>,
964-
ChainResponder<R, McpR>,
965-
>
930+
) -> JrConnectionBuilder<ChainedHandler<H, McpMessageHandler<Role>>, ChainResponder<R, McpR>>
966931
where
967932
H: JrMessageHandler<Role = Role>,
968933
Role: HasEndpoint<Client> + HasEndpoint<Agent>,
@@ -977,12 +942,11 @@ impl<'scope, H: JrMessageHandler, R: JrResponder> JrConnectionBuilder<'scope, H,
977942
pub fn connect_to(
978943
self,
979944
transport: impl Component + 'static,
980-
) -> Result<JrConnection<'scope, H, R>, crate::Error> {
945+
) -> Result<JrConnection<H, R>, crate::Error> {
981946
let Self {
982947
name,
983948
handler,
984949
responder,
985-
pending_tasks,
986950
} = self;
987951

988952
let (outgoing_tx, outgoing_rx) = mpsc::unbounded();
@@ -1007,7 +971,6 @@ impl<'scope, H: JrMessageHandler, R: JrResponder> JrConnectionBuilder<'scope, H,
1007971
name,
1008972
outgoing_rx,
1009973
new_task_rx,
1010-
pending_tasks,
1011974
transport_outgoing_tx,
1012975
transport_incoming_rx,
1013976
dynamic_handler_rx,
@@ -1132,20 +1095,19 @@ impl<'scope, H: JrMessageHandler, R: JrResponder> JrConnectionBuilder<'scope, H,
11321095
///
11331096
/// Most users won't construct this directly - instead use `JrConnectionBuilder::connect_to()` or
11341097
/// `JrConnectionBuilder::serve()` for convenience.
1135-
pub struct JrConnection<'scope, H: JrMessageHandler, R: JrResponder = NullResponder> {
1098+
pub struct JrConnection<H: JrMessageHandler, R: JrResponder<H::Role> = NullResponder> {
11361099
cx: JrConnectionCx<H::Role>,
11371100
name: Option<String>,
11381101
outgoing_rx: mpsc::UnboundedReceiver<OutgoingMessage>,
1139-
new_task_rx: mpsc::UnboundedReceiver<Task<'static>>,
1140-
pending_tasks: Vec<PendingTask<'scope, H::Role>>,
1102+
new_task_rx: mpsc::UnboundedReceiver<Task>,
11411103
transport_outgoing_tx: mpsc::UnboundedSender<Result<jsonrpcmsg::Message, crate::Error>>,
11421104
transport_incoming_rx: mpsc::UnboundedReceiver<Result<jsonrpcmsg::Message, crate::Error>>,
11431105
dynamic_handler_rx: mpsc::UnboundedReceiver<DynamicHandlerMessage<H::Role>>,
11441106
handler: H,
11451107
responder: R,
11461108
}
11471109

1148-
impl<'scope, H: JrMessageHandler, R: JrResponder> JrConnection<'scope, H, R> {
1110+
impl<H: JrMessageHandler, R: JrResponder<H::Role>> JrConnection<H, R> {
11491111
/// Run the connection in server mode with the provided transport.
11501112
///
11511113
/// This drives the connection by continuously processing messages from the transport
@@ -1259,7 +1221,6 @@ impl<'scope, H: JrMessageHandler, R: JrResponder> JrConnection<'scope, H, R> {
12591221
name,
12601222
outgoing_rx,
12611223
new_task_rx,
1262-
pending_tasks,
12631224
handler,
12641225
responder,
12651226
transport_outgoing_tx,
@@ -1286,8 +1247,8 @@ impl<'scope, H: JrMessageHandler, R: JrResponder> JrConnection<'scope, H, R> {
12861247
handler,
12871248
),
12881249
reply_actor::reply_actor(reply_rx),
1289-
task_actor::task_actor(new_task_rx, pending_tasks, &cx),
1290-
responder.run(),
1250+
task_actor::task_actor(new_task_rx, &cx),
1251+
responder.run(cx.clone()),
12911252
)?;
12921253
Ok(())
12931254
};
@@ -1433,14 +1394,14 @@ pub struct JrConnectionCx<Role: JrRole> {
14331394
#[expect(dead_code)]
14341395
role: Role,
14351396
message_tx: OutgoingMessageTx,
1436-
task_tx: TaskTx<'static>,
1397+
task_tx: TaskTx,
14371398
dynamic_handler_tx: mpsc::UnboundedSender<DynamicHandlerMessage<Role>>,
14381399
}
14391400

14401401
impl<Role: JrRole> JrConnectionCx<Role> {
14411402
fn new(
14421403
message_tx: mpsc::UnboundedSender<OutgoingMessage>,
1443-
task_tx: mpsc::UnboundedSender<Task<'static>>,
1404+
task_tx: mpsc::UnboundedSender<Task>,
14441405
dynamic_handler_tx: mpsc::UnboundedSender<DynamicHandlerMessage<Role>>,
14451406
) -> Self {
14461407
Self {
@@ -1535,12 +1496,10 @@ impl<Role: JrRole> JrConnectionCx<Role> {
15351496
/// # }
15361497
/// ```
15371498
#[track_caller]
1538-
pub fn spawn_connection<H: JrMessageHandler>(
1499+
pub fn spawn_connection<H: JrMessageHandler, R: JrResponder<H::Role> + 'static>(
15391500
&self,
1540-
connection: JrConnection<'static, H>,
1541-
serve_future: impl FnOnce(
1542-
JrConnection<'static, H>,
1543-
) -> BoxFuture<'static, Result<(), crate::Error>>,
1501+
connection: JrConnection<H, R>,
1502+
serve_future: impl FnOnce(JrConnection<H, R>) -> BoxFuture<'static, Result<(), crate::Error>>,
15441503
) -> Result<JrConnectionCx<H::Role>, crate::Error> {
15451504
let cx = connection.cx.clone();
15461505
let future = serve_future(connection);
@@ -2478,15 +2437,15 @@ impl JrNotification for UntypedMessage {}
24782437
/// by making blocking explicit and encouraging non-blocking patterns.
24792438
pub struct JrResponse<T> {
24802439
method: String,
2481-
task_tx: TaskTx<'static>,
2440+
task_tx: TaskTx,
24822441
response_rx: oneshot::Receiver<Result<serde_json::Value, crate::Error>>,
24832442
to_result: Box<dyn Fn(serde_json::Value) -> Result<T, crate::Error> + Send>,
24842443
}
24852444

24862445
impl JrResponse<serde_json::Value> {
24872446
fn new(
24882447
method: String,
2489-
task_tx: mpsc::UnboundedSender<Task<'static>>,
2448+
task_tx: mpsc::UnboundedSender<Task>,
24902449
response_rx: oneshot::Receiver<Result<serde_json::Value, crate::Error>>,
24912450
) -> Self {
24922451
Self {

0 commit comments

Comments
 (0)