Lifei/acp session setup refactor (#9488)

This commit is contained in:
Lifei Zhou
2026-06-04 13:12:04 +10:00
committed by GitHub
parent 1cc5aa690a
commit dc59e41945
38 changed files with 3096 additions and 1980 deletions
+59 -50
View File
@@ -16,18 +16,21 @@ use fs_err as fs;
use goose::acp::server::AcpProviderFactory;
use goose::config::base::CONFIG_YAML_NAME;
use goose::config::GooseMode;
use goose::conversation::message::Message;
use goose::model::ModelConfig;
use goose::providers::base::{
stream_from_single_message, MessageStream, Provider, ProviderUsage, Usage,
};
use goose::providers::errors::ProviderError;
use goose_test_support::{McpFixture, FAKE_CODE, TEST_IMAGE_B64, TEST_MODEL};
use sqlx::sqlite::SqlitePoolOptions;
use std::sync::Arc;
use std::time::Duration;
const SHELL_TEST_CONTENT: &str = "test-shell-content-98765";
const OPENAI_SESSION_NAME_RESPONSE: &str = r#"data: {"id":"chatcmpl-test","object":"chat.completion.chunk","created":1766229303,"model":"gpt-5-nano","choices":[{"index":0,"delta":{"role":"assistant","content":""},"finish_reason":null}]}
data: {"id":"chatcmpl-test","object":"chat.completion.chunk","created":1766229303,"model":"gpt-5-nano","choices":[{"index":0,"delta":{"content":"Generated Test Title"},"finish_reason":null}]}
data: {"id":"chatcmpl-test","object":"chat.completion.chunk","created":1766229303,"model":"gpt-5-nano","choices":[{"index":0,"delta":{},"finish_reason":"stop"}]}
data: {"id":"chatcmpl-test","object":"chat.completion.chunk","created":1766229303,"model":"gpt-5-nano","choices":[],"usage":{"prompt_tokens":100,"completion_tokens":10,"total_tokens":110}}
data: [DONE]"#;
struct BasicSession<C: Connection> {
conn: C,
@@ -58,46 +61,6 @@ async fn new_basic_session<C: Connection>(config: TestConnectionConfig) -> Basic
BasicSession { conn, session }
}
struct NamingProvider {
model_config: ModelConfig,
}
#[async_trait::async_trait]
impl Provider for NamingProvider {
fn get_name(&self) -> &str {
"naming-test"
}
async fn stream(
&self,
_model_config: &ModelConfig,
_session_id: &str,
system: &str,
_messages: &[Message],
_tools: &[rmcp::model::Tool],
) -> Result<MessageStream, ProviderError> {
let text = if system.contains("four words or less") || system.contains("4 words or less") {
"Generated Test Title"
} else {
"2"
};
Ok(stream_from_single_message(
Message::assistant().with_text(text),
ProviderUsage::new(self.model_config.model_name.clone(), Usage::default()),
))
}
fn get_model_config(&self) -> ModelConfig {
self.model_config.clone()
}
}
fn naming_provider_factory() -> AcpProviderFactory {
Arc::new(|_provider_name, model_config, _extensions, _working_dir| {
Box::pin(async move { Ok(Arc::new(NamingProvider { model_config }) as Arc<dyn Provider>) })
})
}
pub async fn run_list_sessions<C: Connection>() {
let BasicSession { conn, session } =
new_basic_session::<C>(TestConnectionConfig::default()).await;
@@ -119,6 +82,7 @@ pub async fn run_list_sessions<C: Connection>() {
serde_json::Value::Number(2.into()),
);
expected_meta.insert("userSetName".to_string(), serde_json::Value::Bool(false));
expected_meta.insert("hasRecipe".to_string(), serde_json::Value::Bool(false));
assert_eq!(
response,
ListSessionsResponse::new(vec![SessionInfo::new(
@@ -132,9 +96,21 @@ pub async fn run_list_sessions<C: Connection>() {
pub async fn run_session_name_update_notification<C: Connection>() {
let expected_session_id = C::expected_session_id();
let openai = OpenAiFixture::new(vec![], expected_session_id.clone()).await;
let openai = OpenAiFixture::new(
vec![
(
r#"</info-msg>\nwhat should we call this conversation?""#.into(),
include_str!("../acp_test_data/openai_basic.txt"),
),
(
"Generate a short title for the above messages.".into(),
OPENAI_SESSION_NAME_RESPONSE,
),
],
expected_session_id.clone(),
)
.await;
let config = TestConnectionConfig {
provider_factory: Some(naming_provider_factory()),
disable_session_naming: false,
..Default::default()
};
@@ -923,6 +899,39 @@ pub async fn run_new_session_returns_initial_config<C: Connection>() {
assert!(!models.available_models.is_empty());
}
pub async fn run_new_session_uses_current_config_mode<C: Connection>() {
let temp_dir = tempfile::tempdir().unwrap();
let config_path = temp_dir.path().join(goose::config::base::CONFIG_YAML_NAME);
fs::write(
&config_path,
format!("GOOSE_MODEL: {TEST_MODEL}\nGOOSE_PROVIDER: openai\nGOOSE_MODE: approve\n"),
)
.unwrap();
let expected_session_id = C::expected_session_id();
let openai = OpenAiFixture::new(vec![], expected_session_id.clone()).await;
let config = TestConnectionConfig {
goose_mode: GooseMode::Approve,
data_root: temp_dir.path().to_path_buf(),
..Default::default()
};
let mut conn = C::new(config, openai).await;
let global_config_path =
goose::config::paths::Paths::config_dir().join(goose::config::base::CONFIG_YAML_NAME);
fs::write(
&global_config_path,
format!("GOOSE_MODEL: {TEST_MODEL}\nGOOSE_PROVIDER: openai\nGOOSE_MODE: auto\n"),
)
.unwrap();
let SessionData { session, modes, .. } = conn.new_session().await.unwrap();
expected_session_id.set(&session.session_id().0);
assert_eq!(modes.unwrap().current_mode_id, SessionModeId::new("auto"));
}
pub async fn run_config_option_model_set<C: Connection>() {
run_model_set_impl::<C>(SetModelVia::ConfigOption).await;
}
@@ -1328,11 +1337,11 @@ pub async fn run_prompt_model_mismatch<C: Connection>() {
// TODO: add a Responses API mock to OpenAiFixture so we can test with
// responses-routed models like o4-mini here.
let config = TestConnectionConfig {
current_model: "gpt-4.1".to_string(),
current_model: "gpt-4o".to_string(),
..Default::default()
};
// Server starts on gpt-4.1; client is configured with TEST_MODEL.
// Server starts on gpt-4o; client is configured with TEST_MODEL.
// If session_model is seeded from the response, stream() detects the
// mismatch and sends set_model(TEST_MODEL) before prompting.
let BasicSession { conn: _, .. } = new_basic_session::<C>(config).await;
+60 -130
View File
@@ -12,11 +12,29 @@ use goose::model::ModelConfig;
use goose::providers::base::{MessageStream, Provider};
use goose::providers::errors::ProviderError;
use goose_test_support::{EnforceSessionId, IgnoreSessionId};
use serial_test::serial;
use std::path::PathBuf;
use std::sync::{Arc, Mutex};
use std::sync::{Arc, LazyLock, Mutex};
use common_tests::fixtures::OpenAiFixture;
const DEFAULT_ACP_TEST_CONFIG: &str = "GOOSE_MODEL: gpt-4o\nGOOSE_PROVIDER: openai\n";
static ACP_CONFIG_ROOT: LazyLock<tempfile::TempDir> =
LazyLock::new(|| tempfile::tempdir().unwrap());
fn write_acp_global_config(contents: &str) -> PathBuf {
std::env::set_var("GOOSE_PATH_ROOT", ACP_CONFIG_ROOT.path());
let config_dir = goose::config::paths::Paths::config_dir();
std::fs::create_dir_all(&config_dir).unwrap();
std::fs::write(
config_dir.join(goose::config::base::CONFIG_YAML_NAME),
contents,
)
.unwrap();
config_dir
}
struct MockProvider {
name: String,
model_config: ModelConfig,
@@ -75,7 +93,9 @@ fn mock_provider_factory() -> AcpProviderFactory {
}
#[test]
#[serial]
fn test_custom_get_tools() {
write_acp_global_config(DEFAULT_ACP_TEST_CONFIG);
run_test(async move {
let openai = OpenAiFixture::new(vec![], Arc::new(EnforceSessionId::default())).await;
let mut conn = AcpServerConnection::new(TestConnectionConfig::default(), openai).await;
@@ -98,7 +118,9 @@ fn test_custom_get_tools() {
}
#[test]
#[serial]
fn test_custom_get_extensions() {
write_acp_global_config(DEFAULT_ACP_TEST_CONFIG);
run_test(async move {
let openai = OpenAiFixture::new(vec![], Arc::new(EnforceSessionId::default())).await;
let conn = AcpServerConnection::new(TestConnectionConfig::default(), openai).await;
@@ -124,109 +146,9 @@ fn test_custom_get_extensions() {
}
#[test]
fn test_new_session_passes_cwd_to_provider_factory() {
run_test(async move {
let openai = OpenAiFixture::new(vec![], Arc::new(EnforceSessionId::default())).await;
let cwd = tempfile::tempdir().unwrap();
let expected_cwd = cwd.path().to_path_buf();
let captured_cwds = Arc::new(Mutex::new(Vec::<Option<PathBuf>>::new()));
let factory_cwds = Arc::clone(&captured_cwds);
let provider_factory: AcpProviderFactory = Arc::new(
move |provider_name, model_config, _extensions, working_dir| {
factory_cwds.lock().unwrap().push(working_dir);
Box::pin(async move {
Ok(Arc::new(MockProvider {
name: provider_name,
model_config,
recommended_models: Vec::new(),
supported_models: Vec::new(),
}) as Arc<dyn Provider>)
})
},
);
let mut conn = AcpServerConnection::new(
TestConnectionConfig {
cwd: Some(cwd),
provider_factory: Some(provider_factory),
..Default::default()
},
openai,
)
.await;
conn.new_session().await.unwrap();
let captured_cwd = tokio::time::timeout(std::time::Duration::from_secs(1), async {
loop {
if let Some(cwd) = captured_cwds.lock().unwrap().first().cloned() {
break cwd;
}
tokio::time::sleep(std::time::Duration::from_millis(10)).await;
}
})
.await
.expect("provider factory was not called");
assert_eq!(captured_cwd, Some(expected_cwd));
});
}
#[test]
fn test_load_session_passes_load_cwd_to_provider_factory() {
run_test(async move {
let openai = OpenAiFixture::new(vec![], Arc::new(EnforceSessionId::default())).await;
let initial_cwd = tempfile::tempdir().unwrap();
let captured_cwds = Arc::new(Mutex::new(Vec::<Option<PathBuf>>::new()));
let factory_cwds = Arc::clone(&captured_cwds);
let provider_factory: AcpProviderFactory = Arc::new(
move |provider_name, model_config, _extensions, working_dir| {
factory_cwds.lock().unwrap().push(working_dir);
Box::pin(async move {
Ok(Arc::new(MockProvider {
name: provider_name,
model_config,
recommended_models: Vec::new(),
supported_models: Vec::new(),
}) as Arc<dyn Provider>)
})
},
);
let mut conn = AcpServerConnection::new(
TestConnectionConfig {
cwd: Some(initial_cwd),
provider_factory: Some(provider_factory),
..Default::default()
},
openai,
)
.await;
let SessionData { session, .. } = conn.new_session().await.unwrap();
let session_id = session.session_id().0.to_string();
let SessionData {
session: loaded, ..
} = conn.load_session(&session_id, vec![]).await.unwrap();
let expected_cwd = loaded.work_dir();
let captured_cwd = tokio::time::timeout(std::time::Duration::from_secs(1), async {
loop {
if let Some(cwd) = captured_cwds.lock().unwrap().get(1).cloned() {
break cwd;
}
tokio::time::sleep(std::time::Duration::from_millis(10)).await;
}
})
.await
.expect("provider factory was not called for load session");
assert_eq!(captured_cwd, Some(expected_cwd));
});
}
#[test]
#[serial]
fn test_custom_list_builtin_skill_sources() {
write_acp_global_config(DEFAULT_ACP_TEST_CONFIG);
run_test(async move {
let openai = OpenAiFixture::new(vec![], Arc::new(EnforceSessionId::default())).await;
let conn = AcpServerConnection::new(TestConnectionConfig::default(), openai).await;
@@ -260,7 +182,9 @@ fn test_custom_list_builtin_skill_sources() {
}
#[test]
#[serial]
fn test_custom_provider_inventory_includes_metadata() {
write_acp_global_config(DEFAULT_ACP_TEST_CONFIG);
run_test(async {
let openai = OpenAiFixture::new(vec![], Arc::new(EnforceSessionId::default())).await;
let conn = AcpServerConnection::new(TestConnectionConfig::default(), openai).await;
@@ -291,19 +215,16 @@ fn test_custom_provider_inventory_includes_metadata() {
}
#[test]
#[serial]
fn test_custom_preferences_read_save_remove() {
run_test(async {
let data_root = tempfile::tempdir().unwrap();
std::fs::write(
data_root
.path()
.join(goose::config::base::CONFIG_YAML_NAME),
"GOOSE_MODEL: gpt-4o\nGOOSE_PROVIDER: openai\nGOOSE_AUTO_COMPACT_THRESHOLD: 0.7\nVOICE_AUTO_SUBMIT_PHRASES: send it\n",
)
.unwrap();
let config_dir = write_acp_global_config(
"GOOSE_MODEL: gpt-4o\nGOOSE_PROVIDER: openai\nGOOSE_AUTO_COMPACT_THRESHOLD: 0.7\nVOICE_AUTO_SUBMIT_PHRASES: send it\n",
);
run_test(async move {
let openai = OpenAiFixture::new(vec![], Arc::new(EnforceSessionId::default())).await;
let config = TestConnectionConfig {
data_root: data_root.path().to_path_buf(),
data_root: config_dir,
..Default::default()
};
let conn = AcpServerConnection::new(config, openai).await;
@@ -373,7 +294,9 @@ fn test_custom_preferences_read_save_remove() {
}
#[test]
#[serial]
fn test_custom_preferences_save_rejects_invalid_values() {
write_acp_global_config(DEFAULT_ACP_TEST_CONFIG);
run_test(async {
let openai = OpenAiFixture::new(vec![], Arc::new(EnforceSessionId::default())).await;
let conn = AcpServerConnection::new(TestConnectionConfig::default(), openai).await;
@@ -433,17 +356,16 @@ fn test_custom_preferences_save_rejects_invalid_values() {
}
#[test]
#[serial]
fn test_custom_defaults_read() {
run_test(async {
let data_root = tempfile::tempdir().unwrap();
std::fs::write(
data_root.path().join(goose::config::base::CONFIG_YAML_NAME),
"GOOSE_MODEL: claude-3-5-haiku-latest\nGOOSE_PROVIDER: anthropic\n",
)
.unwrap();
let config_dir = write_acp_global_config(
"GOOSE_MODEL: claude-3-5-haiku-latest\nGOOSE_PROVIDER: anthropic\n",
);
run_test(async move {
let openai = OpenAiFixture::new(vec![], Arc::new(EnforceSessionId::default())).await;
let config = TestConnectionConfig {
data_root: data_root.path().to_path_buf(),
data_root: config_dir,
..Default::default()
};
let conn = AcpServerConnection::new(config, openai).await;
@@ -466,21 +388,15 @@ fn test_custom_defaults_read() {
}
#[test]
#[serial]
fn test_custom_dictation_secret_save_delete() {
let root = tempfile::tempdir().unwrap();
let root_path = root.path().to_string_lossy().to_string();
let _env = env_lock::lock_env([
("GOOSE_PATH_ROOT", Some(root_path.as_str())),
("GOOSE_DISABLE_KEYRING", Some("1")),
("GROQ_API_KEY", None::<&str>),
]);
let config_dir = goose::config::paths::Paths::config_dir();
std::fs::create_dir_all(&config_dir).unwrap();
std::fs::write(
config_dir.join(goose::config::base::CONFIG_YAML_NAME),
let config_dir = write_acp_global_config(
"GOOSE_MODEL: gpt-4o\nGOOSE_PROVIDER: openai\nGOOSE_DISABLE_KEYRING: true\n",
)
.unwrap();
);
run_test(async move {
let openai = OpenAiFixture::new(vec![], Arc::new(EnforceSessionId::default())).await;
@@ -570,7 +486,9 @@ fn test_custom_dictation_secret_save_delete() {
}
#[test]
#[serial]
fn test_raw_config_and_secret_methods_are_removed() {
write_acp_global_config(DEFAULT_ACP_TEST_CONFIG);
run_test(async {
let openai = OpenAiFixture::new(vec![], Arc::new(EnforceSessionId::default())).await;
let conn = AcpServerConnection::new(TestConnectionConfig::default(), openai).await;
@@ -590,7 +508,9 @@ fn test_raw_config_and_secret_methods_are_removed() {
}
#[test]
#[serial]
fn test_provider_switching_updates_session_state() {
write_acp_global_config(DEFAULT_ACP_TEST_CONFIG);
run_test(async {
let openai = OpenAiFixture::new(vec![], Arc::new(EnforceSessionId::default())).await;
let config = TestConnectionConfig {
@@ -618,7 +538,9 @@ fn test_provider_switching_updates_session_state() {
}
#[test]
#[serial]
fn test_custom_unknown_method() {
write_acp_global_config(DEFAULT_ACP_TEST_CONFIG);
run_test(async {
let openai = OpenAiFixture::new(vec![], Arc::new(EnforceSessionId::default())).await;
let conn = AcpServerConnection::new(TestConnectionConfig::default(), openai).await;
@@ -629,6 +551,7 @@ fn test_custom_unknown_method() {
}
#[test]
#[serial]
fn test_developer_fs_requests_use_acp_session_id() {
run_test(async {
let seen_session_id = Arc::new(Mutex::new(None::<String>));
@@ -648,9 +571,14 @@ fn test_developer_fs_requests_use_acp_session_id() {
Arc::new(IgnoreSessionId),
)
.await;
let config_dir = write_acp_global_config(&format!(
"GOOSE_MODEL: gpt-4.1\nGOOSE_PROVIDER: openai\nOPENAI_HOST: {}\n",
openai.uri()
));
let config = TestConnectionConfig {
// gpt-5-nano routes to the Responses API; use a Chat Completions
// model so the canned SSE fixtures are parsed correctly.
data_root: config_dir,
current_model: "gpt-4.1".to_string(),
read_text_file: Some(Arc::new(move |req| {
*seen_session_id_clone.lock().unwrap() = Some(req.session_id.0.to_string());
@@ -683,7 +611,9 @@ fn test_developer_fs_requests_use_acp_session_id() {
}
#[test]
#[serial]
fn test_custom_provider_supported_models_lists_raw_provider_models() {
write_acp_global_config(DEFAULT_ACP_TEST_CONFIG);
run_test(async move {
let openai = OpenAiFixture::new(vec![], Arc::new(EnforceSessionId::default())).await;
let provider_factory: AcpProviderFactory =
+29 -4
View File
@@ -23,13 +23,31 @@ use goose::session_context::SESSION_ID_HEADER;
use goose_test_support::{ExpectedSessionId, TEST_MODEL};
use std::collections::VecDeque;
use std::future::Future;
use std::path::PathBuf;
use std::sync::{Arc, Mutex};
use std::path::{Path, PathBuf};
use std::sync::{Arc, LazyLock, Mutex};
use tokio::task::JoinHandle;
use tokio_util::compat::{TokioAsyncReadCompatExt, TokioAsyncWriteCompatExt};
use wiremock::matchers::{method, path};
use wiremock::{Mock, MockServer, ResponseTemplate};
static ACP_TEST_LOCK: LazyLock<Mutex<()>> = LazyLock::new(|| Mutex::new(()));
static ACP_CONFIG_ROOT: LazyLock<tempfile::TempDir> =
LazyLock::new(|| tempfile::tempdir().unwrap());
fn write_global_test_config(config_path: &Path, openai_base_url: &str) {
let contents = fs::read_to_string(config_path).unwrap();
let mut config: serde_yaml::Mapping = serde_yaml::from_str(&contents).unwrap();
config.insert(
serde_yaml::Value::String("OPENAI_HOST".to_string()),
serde_yaml::Value::String(openai_base_url.to_string()),
);
let global_config_dir = Paths::config_dir();
fs::create_dir_all(&global_config_dir).unwrap();
let global_config_path = global_config_dir.join(goose::config::base::CONFIG_YAML_NAME);
fs::write(&global_config_path, serde_yaml::to_string(&config).unwrap()).unwrap();
}
pub struct OpenAiFixture {
_server: MockServer,
base_url: String,
@@ -167,10 +185,14 @@ pub async fn spawn_acp_server_in_process(
if !config_path.exists() {
fs::write(
&config_path,
format!("GOOSE_MODEL: {current_model}\nGOOSE_PROVIDER: openai\n"),
format!(
"GOOSE_MODEL: {current_model}\nGOOSE_PROVIDER: openai\nGOOSE_MODE: {}\n",
goose_mode
),
)
.unwrap();
}
write_global_test_config(&config_path, openai_base_url);
let provider_factory = provider_factory.unwrap_or_else(|| {
let base_url = openai_base_url.to_string();
Arc::new(
@@ -195,7 +217,6 @@ pub async fn spawn_acp_server_in_process(
builtins: builtins.to_vec(),
data_dir: data_root.to_path_buf(),
config_dir: data_root.to_path_buf(),
goose_mode,
disable_session_naming,
goose_platform: GoosePlatform::GooseCli,
additional_source_roots: Vec::new(),
@@ -585,6 +606,10 @@ pub fn run_test<F>(fut: F)
where
F: Future<Output = ()> + Send + 'static,
{
let _guard = ACP_TEST_LOCK.lock().unwrap_or_else(|err| err.into_inner());
if std::env::var_os("GOOSE_PATH_ROOT").is_none() {
std::env::set_var("GOOSE_PATH_ROOT", ACP_CONFIG_ROOT.path());
}
register_builtin_extensions(goose_mcp::BUILTIN_EXTENSIONS.clone());
let handle = std::thread::Builder::new()
+9 -3
View File
@@ -14,9 +14,10 @@ use common_tests::{
run_load_mode, run_load_model, run_load_session_error, run_load_session_mcp,
run_load_session_replays_image_attachment, run_mode_set, run_model_list, run_model_set,
run_model_set_error_session_not_found, run_new_session_returns_initial_config,
run_permission_persistence, run_prompt_basic, run_prompt_error, run_prompt_image,
run_prompt_image_attachment, run_prompt_mcp, run_prompt_model_mismatch, run_prompt_skill,
run_session_name_update_notification, run_shell_terminal_false, run_shell_terminal_true,
run_new_session_uses_current_config_mode, run_permission_persistence, run_prompt_basic,
run_prompt_error, run_prompt_image, run_prompt_image_attachment, run_prompt_mcp,
run_prompt_model_mismatch, run_prompt_skill, run_session_name_update_notification,
run_shell_terminal_false, run_shell_terminal_true,
};
use goose::config::GooseMode;
use goose::conversation::message::Message;
@@ -240,6 +241,11 @@ fn test_new_session_returns_initial_config() {
run_test(async { run_new_session_returns_initial_config::<AcpServerConnection>().await });
}
#[test]
fn test_new_session_uses_current_config_mode() {
run_test(async { run_new_session_uses_current_config_mode::<AcpServerConnection>().await });
}
#[test]
fn test_model_set() {
run_test(async { run_model_set::<AcpServerConnection>().await });