Skip to content

Commit b1fd5f5

Browse files
committed
feat(acp): add ACP methods to support programmatic goose clients
The ACP protocol was missing several capabilities needed by external clients that talk to goosed over the network. Adds: - `_goose/health` — startup health check for clients that poll before accepting traffic - `_goose/session/set_instructions` — injects a system prompt into an active session under the "recipe" key, replacing the two-call HTTP pattern of start_agent + update_from_session - `session/set_model` now accepts an optional `provider` field for full provider+model switching; empty string is filtered out to avoid opaque registry lookup errors - `_goose/session/get` verified to already return token metrics (input_tokens, output_tokens, accumulated_*, model_config) via the Session struct serialization — confirmed with a new test
1 parent 12eac72 commit b1fd5f5

3 files changed

Lines changed: 237 additions & 9 deletions

File tree

crates/goose-acp/src/custom_requests.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,24 @@ pub struct GetExtensionsResponse {
119119
pub warnings: Vec<String>,
120120
}
121121

122+
/// Apply system prompt instructions to an active session.
123+
/// Equivalent to POST /agent/update_from_session in the HTTP API.
124+
#[derive(Debug, Deserialize, JsonSchema)]
125+
pub struct SetSessionInstructionsRequest {
126+
pub session_id: String,
127+
/// Instructions to prepend to the agent's system prompt (e.g. channel context, persona).
128+
pub instructions: String,
129+
}
130+
131+
/// Health check.
132+
#[derive(Debug, Deserialize, JsonSchema)]
133+
pub struct HealthRequest {}
134+
135+
#[derive(Debug, Serialize, JsonSchema)]
136+
pub struct HealthResponse {
137+
pub status: String,
138+
}
139+
122140
/// Empty success response for operations that return no data.
123141
#[derive(Debug, Serialize, JsonSchema)]
124142
pub struct EmptyResponse {}

crates/goose-acp/src/server.rs

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use goose::mcp_utils::ToolResult;
1515
use goose::permission::permission_confirmation::PrincipalType;
1616
use goose::permission::{Permission, PermissionConfirmation};
1717
use goose::providers::base::Provider;
18+
use goose::providers::create as create_provider;
1819
use goose::providers::provider_registry::ProviderConstructor;
1920
use goose::session::session_manager::SessionType;
2021
use goose::session::{Session, SessionManager};
@@ -971,24 +972,38 @@ impl GooseAcpAgent {
971972
&self,
972973
session_id: &str,
973974
model_id: &str,
975+
provider_override: Option<&str>,
974976
) -> Result<SetSessionModelResponse, sacp::Error> {
975977
let config_path = self.config_dir.join(CONFIG_YAML_NAME);
976978
let config = Config::new(&config_path, "goose").map_err(|e| {
977979
sacp::Error::internal_error().data(format!("Failed to read config: {}", e))
978980
})?;
979-
let provider_name = config.get_goose_provider().map_err(|_| {
980-
sacp::Error::internal_error().data("No provider configured".to_string())
981-
})?;
981+
let provider_name = if let Some(p) = provider_override {
982+
p.to_string()
983+
} else {
984+
config.get_goose_provider().map_err(|_| {
985+
sacp::Error::internal_error().data("No provider configured".to_string())
986+
})?
987+
};
982988
let model_config = goose::model::ModelConfig::new(model_id)
983989
.map_err(|e| {
984990
sacp::Error::invalid_params().data(format!("Invalid model config: {}", e))
985991
})?
986992
.with_canonical_limits(&provider_name);
987-
let provider = (self.provider_factory)(model_config, Vec::new())
988-
.await
989-
.map_err(|e| {
990-
sacp::Error::internal_error().data(format!("Failed to create provider: {}", e))
991-
})?;
993+
let provider = if provider_override.is_some() {
994+
// When switching providers, use the global registry (same as HTTP update_agent_provider).
995+
create_provider(&provider_name, model_config, Vec::new())
996+
.await
997+
.map_err(|e| {
998+
sacp::Error::internal_error().data(format!("Failed to create provider: {}", e))
999+
})?
1000+
} else {
1001+
(self.provider_factory)(model_config, Vec::new())
1002+
.await
1003+
.map_err(|e| {
1004+
sacp::Error::internal_error().data(format!("Failed to create provider: {}", e))
1005+
})?
1006+
};
9921007

9931008
let agent = {
9941009
let sessions = self.sessions.lock().await;
@@ -1169,6 +1184,25 @@ impl GooseAcpAgent {
11691184
})
11701185
}
11711186

1187+
#[custom_method("health")]
1188+
async fn on_health(&self, _req: HealthRequest) -> Result<HealthResponse, sacp::Error> {
1189+
Ok(HealthResponse {
1190+
status: "ok".to_string(),
1191+
})
1192+
}
1193+
1194+
#[custom_method("session/set_instructions")]
1195+
async fn on_set_session_instructions(
1196+
&self,
1197+
req: SetSessionInstructionsRequest,
1198+
) -> Result<EmptyResponse, sacp::Error> {
1199+
let agent = self.get_agent_for_session(&req.session_id).await?;
1200+
agent
1201+
.extend_system_prompt("recipe".to_string(), req.instructions)
1202+
.await;
1203+
Ok(EmptyResponse {})
1204+
}
1205+
11721206
#[custom_method("config/extensions")]
11731207
async fn on_get_extensions(&self) -> Result<GetExtensionsResponse, sacp::Error> {
11741208
let extensions = goose::config::extensions::get_all_extensions();
@@ -1276,12 +1310,24 @@ impl JrMessageHandler for GooseAcpHandler {
12761310
MessageCx::Request(req, request_cx)
12771311
if req.method == "session/set_model" =>
12781312
{
1313+
// Extract optional `provider` before consuming params (sacp's
1314+
// SetSessionModelRequest doesn't have this field).
1315+
let provider_override = req
1316+
.params
1317+
.get("provider")
1318+
.and_then(|v| v.as_str())
1319+
.filter(|s| !s.is_empty())
1320+
.map(String::from);
12791321
let params: SetSessionModelRequest =
12801322
serde_json::from_value(req.params).map_err(|e| {
12811323
sacp::Error::invalid_params().data(e.to_string())
12821324
})?;
12831325
let resp = agent
1284-
.on_set_model(&params.session_id.0, &params.model_id.0)
1326+
.on_set_model(
1327+
&params.session_id.0,
1328+
&params.model_id.0,
1329+
provider_override.as_deref(),
1330+
)
12851331
.await?;
12861332
let json = serde_json::to_value(resp).map_err(|e| {
12871333
sacp::Error::internal_error().data(e.to_string())

crates/goose-acp/tests/custom_requests_test.rs

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,3 +168,167 @@ fn test_custom_unknown_method() {
168168
assert!(result.is_err(), "expected method_not_found error");
169169
});
170170
}
171+
172+
#[test]
173+
fn test_custom_health() {
174+
run_test(async {
175+
let openai = OpenAiFixture::new(vec![], ExpectedSessionId::default()).await;
176+
let conn = ClientToAgentConnection::new(TestConnectionConfig::default(), openai).await;
177+
178+
let result = send_custom(conn.cx(), "_goose/health", serde_json::json!({})).await;
179+
assert!(result.is_ok(), "expected ok, got: {:?}", result);
180+
181+
let response = result.unwrap();
182+
assert_eq!(
183+
response.get("status").and_then(|v| v.as_str()),
184+
Some("ok"),
185+
"expected status 'ok'"
186+
);
187+
});
188+
}
189+
190+
#[test]
191+
fn test_custom_set_session_instructions() {
192+
run_test(async {
193+
let openai = OpenAiFixture::new(vec![], ExpectedSessionId::default()).await;
194+
let mut conn = ClientToAgentConnection::new(TestConnectionConfig::default(), openai).await;
195+
196+
let (session, _models) = conn.new_session().await;
197+
let session_id = session.session_id().0.clone();
198+
199+
let result = send_custom(
200+
conn.cx(),
201+
"_goose/session/set_instructions",
202+
serde_json::json!({
203+
"session_id": session_id,
204+
"instructions": "You are a helpful assistant for the #eng-platform Slack channel.",
205+
}),
206+
)
207+
.await;
208+
assert!(result.is_ok(), "set_instructions failed: {:?}", result);
209+
});
210+
}
211+
212+
#[test]
213+
fn test_custom_set_session_instructions_unknown_session() {
214+
run_test(async {
215+
let openai = OpenAiFixture::new(vec![], ExpectedSessionId::default()).await;
216+
let conn = ClientToAgentConnection::new(TestConnectionConfig::default(), openai).await;
217+
218+
let result = send_custom(
219+
conn.cx(),
220+
"_goose/session/set_instructions",
221+
serde_json::json!({
222+
"session_id": "nonexistent-session-id",
223+
"instructions": "some instructions",
224+
}),
225+
)
226+
.await;
227+
assert!(result.is_err(), "expected error for unknown session");
228+
});
229+
}
230+
231+
#[test]
232+
fn test_custom_session_get_includes_token_fields() {
233+
run_test(async {
234+
let openai = OpenAiFixture::new(vec![], ExpectedSessionId::default()).await;
235+
let mut conn = ClientToAgentConnection::new(TestConnectionConfig::default(), openai).await;
236+
237+
let (session, _models) = conn.new_session().await;
238+
let session_id = session.session_id().0.clone();
239+
240+
let result = send_custom(
241+
conn.cx(),
242+
"_goose/session/get",
243+
serde_json::json!({ "session_id": session_id }),
244+
)
245+
.await;
246+
assert!(result.is_ok(), "expected ok, got: {:?}", result);
247+
248+
let response = result.unwrap();
249+
let returned_session = response.get("session").expect("missing 'session' field");
250+
251+
// Verify token metric fields are present (may be null for a fresh session).
252+
assert!(
253+
returned_session.get("input_tokens").is_some(),
254+
"missing 'input_tokens' field"
255+
);
256+
assert!(
257+
returned_session.get("output_tokens").is_some(),
258+
"missing 'output_tokens' field"
259+
);
260+
assert!(
261+
returned_session.get("accumulated_total_tokens").is_some(),
262+
"missing 'accumulated_total_tokens' field"
263+
);
264+
assert!(
265+
returned_session.get("accumulated_input_tokens").is_some(),
266+
"missing 'accumulated_input_tokens' field"
267+
);
268+
assert!(
269+
returned_session.get("accumulated_output_tokens").is_some(),
270+
"missing 'accumulated_output_tokens' field"
271+
);
272+
273+
// model_config contains model_name and context_limit (the slackbot reads context_limit
274+
// via provider_config.context_limit in the HTTP API equivalent).
275+
// For a fresh session, model_config is populated from the configured provider.
276+
assert!(
277+
returned_session.get("model_config").is_some(),
278+
"missing 'model_config' field"
279+
);
280+
});
281+
}
282+
283+
#[test]
284+
fn test_session_set_model_with_provider() {
285+
run_test(async {
286+
let openai = OpenAiFixture::new(vec![], ExpectedSessionId::default()).await;
287+
let mut conn = ClientToAgentConnection::new(TestConnectionConfig::default(), openai).await;
288+
289+
let (session, _models) = conn.new_session().await;
290+
let session_id = session.session_id().0.clone();
291+
292+
// Switch model without specifying provider (reads from config — same as before).
293+
// Uses camelCase per sacp's SetSessionModelRequest field naming.
294+
let result = send_custom(
295+
conn.cx(),
296+
"session/set_model",
297+
serde_json::json!({
298+
"sessionId": session_id,
299+
"modelId": "gpt-4o",
300+
}),
301+
)
302+
.await;
303+
assert!(result.is_ok(), "set_model failed: {:?}", result);
304+
});
305+
}
306+
307+
#[test]
308+
fn test_session_set_model_with_explicit_provider() {
309+
run_test(async {
310+
let openai = OpenAiFixture::new(vec![], ExpectedSessionId::default()).await;
311+
let mut conn = ClientToAgentConnection::new(TestConnectionConfig::default(), openai).await;
312+
313+
let (session, _models) = conn.new_session().await;
314+
let session_id = session.session_id().0.clone();
315+
316+
// Switch provider + model explicitly — the extra `provider` field is extracted from raw
317+
// params before the sacp-typed parse.
318+
let result = send_custom(
319+
conn.cx(),
320+
"session/set_model",
321+
serde_json::json!({
322+
"sessionId": session_id,
323+
"modelId": "gpt-4o",
324+
"provider": "openai",
325+
}),
326+
)
327+
.await;
328+
assert!(
329+
result.is_ok(),
330+
"set_model with provider failed: {:?}",
331+
result
332+
);
333+
});
334+
}

0 commit comments

Comments
 (0)