diff --git a/crates/goose-provider-types/src/base.rs b/crates/goose-provider-types/src/base.rs index f8cad1708..8b55e37e4 100644 --- a/crates/goose-provider-types/src/base.rs +++ b/crates/goose-provider-types/src/base.rs @@ -5,7 +5,6 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use std::collections::HashMap; use std::pin::Pin; -use std::sync::Arc; use crate::{ canonical::{map_to_canonical_model, CanonicalModelRegistry}, @@ -591,8 +590,6 @@ pub trait Provider: Send + Sync { false } - fn set_session_title_callback(&self, _callback: Arc) {} - /// Configure OAuth authentication for this provider /// /// This method is called when a provider has configuration keys marked with oauth_flow = true. diff --git a/crates/goose/src/acp/provider.rs b/crates/goose/src/acp/provider.rs index a5fbade23..0f1c98586 100644 --- a/crates/goose/src/acp/provider.rs +++ b/crates/goose/src/acp/provider.rs @@ -149,30 +149,6 @@ struct HandoffContextClaim { include_context: bool, } -type SessionTitleCallback = Arc; - -#[derive(Clone, Default)] -struct SessionTitlePublisher { - callback: Arc>>, -} - -impl SessionTitlePublisher { - fn set_callback(&self, callback: SessionTitleCallback) { - *self.callback.lock().unwrap() = Some(callback); - } - - fn publish(&self, title: &str) { - let title = title.trim(); - if title.is_empty() { - return; - } - - if let Some(callback) = self.callback.lock().unwrap().clone() { - callback(title.to_string()); - } - } -} - pub struct AcpProvider { name: String, goose_mode: Arc>, @@ -189,7 +165,6 @@ pub struct AcpProvider { /// in which case `get_context_limit()` falls back to the supplied model /// configuration's context limit. context_size: Arc, - session_title_publisher: SessionTitlePublisher, /// Config option id used to select the model, if this agent supports it. model_config_option_id: Option, @@ -277,13 +252,11 @@ impl AcpProvider { let pending_tool_updates: Arc>> = Arc::new(Mutex::new(HashMap::new())); let context_size = Arc::new(AtomicU64::new(0)); - let session_title_publisher = SessionTitlePublisher::default(); let client_loop = AcpClientLoop::new( config, goose_mode_shared.clone(), pending_tool_updates.clone(), context_size.clone(), - session_title_publisher.clone(), ); let loop_thread = spawn_client_loop(run(client_loop, rx, init_tx)); @@ -316,7 +289,6 @@ impl AcpProvider { pending_tool_updates, handoff_context_sent: AtomicBool::new(false), context_size, - session_title_publisher, model_config_option_id, applied_model: Arc::new(Mutex::new(applied_model)), tx: Some(tx), @@ -542,10 +514,6 @@ impl Provider for AcpProvider { true } - fn set_session_title_callback(&self, callback: Arc) { - self.session_title_publisher.set_callback(callback); - } - async fn handle_permission_confirmation( &self, request_id: &str, @@ -786,7 +754,6 @@ struct AcpClientLoop { prompt_response_tx: Arc>>>, pending_tool_updates: Arc>>, context_size: Arc, - session_title_publisher: SessionTitlePublisher, } impl AcpClientLoop { @@ -795,7 +762,6 @@ impl AcpClientLoop { goose_mode: Arc>, pending_tool_updates: Arc>>, context_size: Arc, - session_title_publisher: SessionTitlePublisher, ) -> Self { Self { config, @@ -803,7 +769,6 @@ impl AcpClientLoop { prompt_response_tx: Arc::new(Mutex::new(None)), pending_tool_updates, context_size, - session_title_publisher, } } @@ -858,7 +823,6 @@ impl AcpClientLoop { prompt_response_tx, pending_tool_updates, context_size, - session_title_publisher, } = self; let notification_callback = config.notification_callback.clone(); let reverse_modes = reverse_mode_mapping(&config.mode_mapping); @@ -872,7 +836,6 @@ impl AcpClientLoop { let goose_mode = goose_mode.clone(); let pending_tool_updates = pending_tool_updates.clone(); let context_size = context_size.clone(); - let session_title_publisher = session_title_publisher.clone(); async move |notification: SessionNotification, _cx| { if let Some(ref cb) = notification_callback { cb(notification.clone()); @@ -909,11 +872,6 @@ impl AcpClientLoop { SessionUpdate::UsageUpdate(usage) => { context_size.store(usage.size, Ordering::Relaxed); } - SessionUpdate::SessionInfoUpdate(update) => { - if let Some(title) = update.title.value() { - session_title_publisher.publish(title); - } - } _ => {} } if let Some(tx) = prompt_response_tx @@ -1858,21 +1816,6 @@ mod tests { test_provider_with_tx(None) } - #[test] - fn session_title_publisher_forwards_non_empty_titles() { - let publisher = SessionTitlePublisher::default(); - let titles = Arc::new(Mutex::new(Vec::new())); - let received = titles.clone(); - publisher.set_callback(Arc::new(move |title| { - received.lock().unwrap().push(title); - })); - - publisher.publish(" Generated title "); - publisher.publish(" "); - - assert_eq!(*titles.lock().unwrap(), vec!["Generated title"]); - } - fn test_provider_with_tx( tx: Option>, ) -> (AcpProvider, ModelConfig) { @@ -1889,7 +1832,6 @@ mod tests { pending_tool_updates: Arc::new(Mutex::new(HashMap::new())), handoff_context_sent: AtomicBool::new(false), context_size: Arc::new(AtomicU64::new(0)), - session_title_publisher: SessionTitlePublisher::default(), model_config_option_id: None, applied_model: Arc::new(Mutex::new(None)), tx, diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index e93199ee9..af8a4154c 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -3406,29 +3406,6 @@ impl Agent { session_id: &str, ) -> Result<()> { let provider_name = provider.get_name().to_string(); - let session_manager = self.config.session_manager.clone(); - let session_name_update_tx = self.config.session_name_update_tx.clone(); - let session_id_for_title = session_id.to_string(); - let runtime = tokio::runtime::Handle::current(); - provider.set_session_title_callback(Arc::new(move |title| { - let session_manager = session_manager.clone(); - let session_name_update_tx = session_name_update_tx.clone(); - let session_id = session_id_for_title.clone(); - runtime.spawn(async move { - match session_manager - .update_name_from_provider(&session_id, title) - .await - { - Ok(Some(update)) => { - if let Some(tx) = session_name_update_tx { - let _ = tx.send(update); - } - } - Ok(None) => {} - Err(error) => warn!(%error, "Failed to apply provider session title"), - } - }); - })); // Normalize against the provider entry so custom/declarative providers // backfill `context_limit` from their known models before the config is diff --git a/crates/goose/src/session/session_manager.rs b/crates/goose/src/session/session_manager.rs index e1f0cb1b5..5adf1f7d1 100644 --- a/crates/goose/src/session/session_manager.rs +++ b/crates/goose/src/session/session_manager.rs @@ -557,24 +557,6 @@ impl SessionManager { }) } - pub async fn update_name_from_provider( - &self, - id: &str, - name: String, - ) -> Result> { - let name = name.trim().to_string(); - if name.is_empty() { - return Ok(None); - } - - let session = self.get_session(id, false).await?; - if session.user_set_name || session.name == name { - return Ok(None); - } - - Ok(Some(self.system_generated_name_update(id, name).await?)) - } - pub async fn maybe_update_name( &self, id: &str, @@ -3221,60 +3203,6 @@ mod tests { assert_eq!(update.name, "investigate session naming with"); } - #[tokio::test] - async fn test_provider_name_replaces_generated_name_but_not_user_name() { - let temp_dir = TempDir::new().unwrap(); - let sm = SessionManager::new(temp_dir.path().to_path_buf()); - let session = sm - .create_session( - temp_dir.path().to_path_buf(), - "Local fallback".to_string(), - SessionType::User, - GooseMode::default(), - ) - .await - .unwrap(); - - let update = sm - .update_name_from_provider(&session.id, " Better ACP title ".to_string()) - .await - .unwrap() - .unwrap(); - assert_eq!(update.name, "Better ACP title"); - - sm.update(&session.id) - .model_config(ModelConfig::new("test-model")) - .apply() - .await - .unwrap(); - add_user_message(&sm, &session.id).await; - add_user_message(&sm, &session.id).await; - assert!(sm - .maybe_update_name(&session.id, Arc::new(StatefulNamingTestProvider)) - .await - .unwrap() - .is_none()); - assert_eq!( - sm.get_session(&session.id, false).await.unwrap().name, - "Better ACP title" - ); - - sm.update(&session.id) - .user_provided_name("Manual title") - .apply() - .await - .unwrap(); - assert!(sm - .update_name_from_provider(&session.id, "Another ACP title".to_string()) - .await - .unwrap() - .is_none()); - assert_eq!( - sm.get_session(&session.id, false).await.unwrap().name, - "Manual title" - ); - } - #[tokio::test] async fn test_maybe_update_name_preserves_user_renamed_session() { let temp_dir = TempDir::new().unwrap();