Skip to content

Commit 1d49fcf

Browse files
nikomatsakisnikomatsakis
authored
Merge pull request #26 from nikomatsakis/main
cleanup the component trait
2 parents 56949bc + c0a5405 commit 1d49fcf

File tree

12 files changed

+298
-305
lines changed

12 files changed

+298
-305
lines changed

‎src/sacp-conductor/src/conductor.rs‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,6 @@ use std::{
120120
use futures::{
121121
SinkExt, StreamExt,
122122
channel::mpsc::{self},
123-
future::BoxFuture,
124123
};
125124
use sacp::{
126125
JrConnectionCx, JrHandlerChain, JrNotification, JrRequest, JrRequestCx, JrResponse,
@@ -224,11 +223,8 @@ impl Conductor {
224223
}
225224

226225
impl sacp::Component for Conductor {
227-
fn serve(
228-
self: Box<Self>,
229-
channels: sacp::Channels,
230-
) -> BoxFuture<'static, Result<(), sacp::Error>> {
231-
Box::pin(async move { (*self).run(channels).await })
226+
async fn serve(self, channels: sacp::Channels) -> Result<(), sacp::Error> {
227+
self.run(channels).await
232228
}
233229
}
234230

@@ -875,9 +871,9 @@ pub enum SourceComponentIndex {
875871
///
876872
/// Simple case - provide all components unconditionally:
877873
/// ```ignore
878-
/// let components: Vec<Box<dyn Component>> = vec![
879-
/// Box::new(AcpAgent::from_str("python proxy.py")?),
880-
/// Box::new(AcpAgent::from_str("python agent.py")?),
874+
/// let components: Vec<sacp::DynComponent> = vec![
875+
/// sacp::DynComponent::new(AcpAgent::from_str("python proxy.py")?),
876+
/// sacp::DynComponent::new(AcpAgent::from_str("python agent.py")?),
881877
/// ];
882878
/// Conductor::new("my-conductor", components, None)
883879
/// ```
@@ -886,11 +882,11 @@ pub enum SourceComponentIndex {
886882
/// ```ignore
887883
/// Conductor::new("my-conductor", |_cx, _conductor_tx, init_req| async move {
888884
/// let needs_auth = init_req.capabilities.contains(&"auth");
889-
/// let mut components: Vec<Box<dyn Component>> = Vec::new();
885+
/// let mut components: Vec<sacp::DynComponent> = Vec::new();
890886
/// if needs_auth {
891-
/// components.push(Box::new(AcpAgent::from_str("python auth-proxy.py")?));
887+
/// components.push(sacp::DynComponent::new(AcpAgent::from_str("python auth-proxy.py")?));
892888
/// }
893-
/// components.push(Box::new(AcpAgent::from_str("python agent.py")?));
889+
/// components.push(sacp::DynComponent::new(AcpAgent::from_str("python agent.py")?));
894890
/// Ok((init_req, components))
895891
/// }, None)
896892
/// ```
@@ -911,7 +907,7 @@ pub trait ComponentList: Send {
911907
req: InitializeRequest,
912908
) -> futures::future::BoxFuture<
913909
'static,
914-
Result<(InitializeRequest, Vec<Box<dyn Component>>), sacp::Error>,
910+
Result<(InitializeRequest, Vec<sacp::DynComponent>), sacp::Error>,
915911
>;
916912
}
917913

@@ -925,12 +921,12 @@ where
925921
req: InitializeRequest,
926922
) -> futures::future::BoxFuture<
927923
'static,
928-
Result<(InitializeRequest, Vec<Box<dyn Component>>), sacp::Error>,
924+
Result<(InitializeRequest, Vec<sacp::DynComponent>), sacp::Error>,
929925
> {
930926
Box::pin(async move {
931-
let components: Vec<Box<dyn Component>> = (*self)
927+
let components: Vec<sacp::DynComponent> = (*self)
932928
.into_iter()
933-
.map(|c| Box::new(c) as Box<dyn Component>)
929+
.map(|c| sacp::DynComponent::new(c))
934930
.collect();
935931
Ok((req, components))
936932
})
@@ -942,7 +938,7 @@ impl<F, Fut> ComponentList for F
942938
where
943939
F: FnOnce(InitializeRequest) -> Fut + Send + 'static,
944940
Fut: std::future::Future<
945-
Output = Result<(InitializeRequest, Vec<Box<dyn Component>>), sacp::Error>,
941+
Output = Result<(InitializeRequest, Vec<sacp::DynComponent>), sacp::Error>,
946942
> + Send
947943
+ 'static,
948944
{
@@ -951,7 +947,7 @@ where
951947
req: InitializeRequest,
952948
) -> futures::future::BoxFuture<
953949
'static,
954-
Result<(InitializeRequest, Vec<Box<dyn Component>>), sacp::Error>,
950+
Result<(InitializeRequest, Vec<sacp::DynComponent>), sacp::Error>,
955951
> {
956952
Box::pin(async move { (*self)(req).await })
957953
}

‎src/sacp-conductor/tests/initialization_sequence.rs‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,21 +61,18 @@ struct InitComponent {
6161
}
6262

6363
impl InitComponent {
64-
fn new(config: &Arc<InitConfig>) -> Box<dyn Component> {
65-
Box::new(Self {
64+
fn new(config: &Arc<InitConfig>) -> sacp::DynComponent {
65+
sacp::DynComponent::new(Self {
6666
config: config.clone(),
6767
})
6868
}
6969
}
7070

7171
impl Component for InitComponent {
72-
fn serve(
73-
self: Box<Self>,
74-
channels: Channels,
75-
) -> sacp::BoxFuture<'static, Result<(), sacp::Error>> {
72+
async fn serve(self, channels: Channels) -> Result<(), sacp::Error> {
7673
let config = Arc::clone(&self.config);
7774

78-
Box::pin(async move {
75+
{
7976
JrHandlerChain::new()
8077
.name("init-component")
8178
.on_receive_request(async move |mut request: InitializeRequest, request_cx| {
@@ -110,12 +107,12 @@ impl Component for InitComponent {
110107
})
111108
.serve(channels)
112109
.await
113-
})
110+
}
114111
}
115112
}
116113

117114
async fn run_test_with_components(
118-
components: Vec<Box<dyn Component>>,
115+
components: Vec<sacp::DynComponent>,
119116
editor_task: impl AsyncFnOnce(sacp::JrConnectionCx) -> Result<(), sacp::Error>,
120117
) -> Result<(), sacp::Error> {
121118
// Set up editor <-> conductor communication

‎src/sacp-conductor/tests/mcp-integration.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ fn conductor_command() -> Vec<String> {
4242
}
4343

4444
async fn run_test_with_components(
45-
components: Vec<Box<dyn Component>>,
45+
components: Vec<sacp::DynComponent>,
4646
editor_task: impl AsyncFnOnce(sacp::JrConnectionCx) -> Result<(), sacp::Error>,
4747
) -> Result<(), sacp::Error> {
4848
// Set up editor <-> conductor communication

‎src/sacp-conductor/tests/mcp_integration/agent.rs‎

Lines changed: 73 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -22,83 +22,81 @@ struct AgentState {
2222
}
2323

2424
impl Component for AgentComponent {
25-
fn serve(self: Box<Self>, channels: Channels) -> BoxFuture<'static, Result<(), sacp::Error>> {
26-
Box::pin(async move {
27-
let state = AgentState {
28-
mcp_servers: Arc::new(Mutex::new(Vec::new())),
29-
};
30-
31-
JrHandlerChain::new()
32-
.name("agent-component")
33-
.on_receive_request(async move |request: InitializeRequest, request_cx| {
34-
// Simple initialization response
35-
let response = InitializeResponse {
36-
protocol_version: request.protocol_version,
37-
agent_capabilities: AgentCapabilities::default(),
38-
auth_methods: vec![],
39-
meta: None,
40-
agent_info: None,
41-
};
42-
request_cx.respond(response)
43-
})
44-
.on_receive_request({
45-
let state = state.clone();
46-
async move |request: NewSessionRequest, request_cx| {
47-
assert_eq!(request.mcp_servers.len(), 1);
48-
49-
// Although the proxy injects an HTTP server, it will be rewritten to stdio by the conductor.
50-
let mcp_server = &request.mcp_servers[0];
51-
assert!(
52-
matches!(mcp_server, McpServer::Stdio { .. }),
53-
"expected a stdio MCP server: {:?}",
54-
request.mcp_servers
55-
);
56-
57-
// Verify the stdio configuration is correct
58-
if let McpServer::Stdio {
59-
name,
60-
command,
61-
args,
62-
..
63-
} = mcp_server
64-
{
65-
assert_eq!(name, "test");
66-
let conductor_command = conductor_command();
67-
assert_eq!(command.to_str().unwrap(), &conductor_command[0]);
68-
for (arg, expected) in args.iter().zip(&conductor_command[1..]) {
69-
assert_eq!(arg, expected);
70-
}
25+
async fn serve(self, channels: Channels) -> Result<(), sacp::Error> {
26+
let state = AgentState {
27+
mcp_servers: Arc::new(Mutex::new(Vec::new())),
28+
};
29+
30+
JrHandlerChain::new()
31+
.name("agent-component")
32+
.on_receive_request(async move |request: InitializeRequest, request_cx| {
33+
// Simple initialization response
34+
let response = InitializeResponse {
35+
protocol_version: request.protocol_version,
36+
agent_capabilities: AgentCapabilities::default(),
37+
auth_methods: vec![],
38+
meta: None,
39+
agent_info: None,
40+
};
41+
request_cx.respond(response)
42+
})
43+
.on_receive_request({
44+
let state = state.clone();
45+
async move |request: NewSessionRequest, request_cx| {
46+
assert_eq!(request.mcp_servers.len(), 1);
47+
48+
// Although the proxy injects an HTTP server, it will be rewritten to stdio by the conductor.
49+
let mcp_server = &request.mcp_servers[0];
50+
assert!(
51+
matches!(mcp_server, McpServer::Stdio { .. }),
52+
"expected a stdio MCP server: {:?}",
53+
request.mcp_servers
54+
);
55+
56+
// Verify the stdio configuration is correct
57+
if let McpServer::Stdio {
58+
name,
59+
command,
60+
args,
61+
..
62+
} = mcp_server
63+
{
64+
assert_eq!(name, "test");
65+
let conductor_command = conductor_command();
66+
assert_eq!(command.to_str().unwrap(), &conductor_command[0]);
67+
for (arg, expected) in args.iter().zip(&conductor_command[1..]) {
68+
assert_eq!(arg, expected);
7169
}
70+
}
7271

73-
// Store MCP servers for later use
74-
*state.mcp_servers.lock().await = request.mcp_servers;
72+
// Store MCP servers for later use
73+
*state.mcp_servers.lock().await = request.mcp_servers;
7574

76-
// Simple session response
77-
let response = NewSessionResponse {
78-
session_id: "test-session-123".into(),
79-
modes: None,
80-
meta: None,
81-
};
82-
request_cx.respond(response)
83-
}
84-
})
85-
.on_receive_request({
75+
// Simple session response
76+
let response = NewSessionResponse {
77+
session_id: "test-session-123".into(),
78+
modes: None,
79+
meta: None,
80+
};
81+
request_cx.respond(response)
82+
}
83+
})
84+
.on_receive_request({
85+
let state = state.clone();
86+
async move |request: PromptRequest, request_cx| {
87+
tracing::debug!(
88+
session_id = %request.session_id.0,
89+
"Received prompt request"
90+
);
91+
92+
// Run the rest out of turn so the loop stays responsive
93+
let connection_cx = request_cx.connection_cx();
8694
let state = state.clone();
87-
async move |request: PromptRequest, request_cx| {
88-
tracing::debug!(
89-
session_id = %request.session_id.0,
90-
"Received prompt request"
91-
);
92-
93-
// Run the rest out of turn so the loop stays responsive
94-
let connection_cx = request_cx.connection_cx();
95-
let state = state.clone();
96-
connection_cx.spawn(Self::respond_to_prompt(state, request, request_cx))
97-
}
98-
})
99-
.serve(channels)
100-
.await
101-
})
95+
connection_cx.spawn(Self::respond_to_prompt(state, request, request_cx))
96+
}
97+
})
98+
.serve(channels)
99+
.await
102100
}
103101
}
104102

@@ -205,6 +203,6 @@ impl AgentComponent {
205203
}
206204
}
207205

208-
pub fn create() -> Box<dyn Component> {
209-
Box::new(AgentComponent)
206+
pub fn create() -> sacp::DynComponent {
207+
sacp::DynComponent::new(AgentComponent)
210208
}

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

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,18 @@ use crate::mcp_integration::mcp_server::TestMcpServer;
88
pub struct ProxyComponent;
99

1010
impl Component for ProxyComponent {
11-
fn serve(self: Box<Self>, channels: Channels) -> BoxFuture<'static, Result<(), sacp::Error>> {
12-
Box::pin(async move {
13-
JrHandlerChain::new()
14-
.name("proxy-component")
15-
.provide_mcp(
16-
McpServiceRegistry::default().with_rmcp_server("test", TestMcpServer::new)?,
17-
)
18-
.proxy()
19-
.serve(channels)
20-
.await
21-
})
11+
async fn serve(self, channels: Channels) -> Result<(), sacp::Error> {
12+
JrHandlerChain::new()
13+
.name("proxy-component")
14+
.provide_mcp(
15+
McpServiceRegistry::default().with_rmcp_server("test", TestMcpServer::new)?,
16+
)
17+
.proxy()
18+
.serve(channels)
19+
.await
2220
}
2321
}
2422

25-
pub fn create() -> Box<dyn Component> {
26-
Box::new(ProxyComponent)
23+
pub fn create() -> sacp::DynComponent {
24+
sacp::DynComponent::new(ProxyComponent)
2725
}

0 commit comments

Comments
 (0)