From f09f1d788d7edac7a009aeee428d0074fc213154 Mon Sep 17 00:00:00 2001 From: Daniel Kurzynski Date: Fri, 6 Feb 2026 11:14:32 +0100 Subject: [PATCH] Fix empty session cleanup when multiple windows opened in parallel Defer session persistence until the first prompt arrives instead of creating sessions immediately in new_session(). This prevents empty sessions from being created when the client opens multiple windows simultaneously but the user only interacts with one. - Add PendingSession struct to hold session config before materialization - Store pending sessions in a HashMap keyed by session ID - Materialize pending sessions on first prompt() call - Add create_session_with_id() to SessionManager for deferred creation --- crates/code_assistant/src/acp/agent.rs | 92 ++++++++++++-------- crates/code_assistant/src/session/manager.rs | 11 +++ 2 files changed, 68 insertions(+), 35 deletions(-) diff --git a/crates/code_assistant/src/acp/agent.rs b/crates/code_assistant/src/acp/agent.rs index a97d584f..aea3a2cc 100644 --- a/crates/code_assistant/src/acp/agent.rs +++ b/crates/code_assistant/src/acp/agent.rs @@ -20,6 +20,13 @@ use command_executor::{CommandExecutor, DefaultCommandExecutor}; use llm::factory::create_llm_client_from_model; use llm::provider_config::ConfigurationSystem; +/// Pending session that hasn't been persisted yet (deferred until first prompt). +#[derive(Clone)] +struct PendingSession { + config: SessionConfig, + model_config: SessionModelConfig, +} + /// Global connection to the ACP client /// Since there's only one connection per agent process, this is acceptable static ACP_CLIENT_CONNECTION: OnceLock> = OnceLock::new(); @@ -47,6 +54,8 @@ pub struct ACPAgentImpl { /// Used to signal cancellation to the prompt() wait loop active_uis: Arc>>>, client_capabilities: Arc>>, + /// Sessions created in new_session() but not yet persisted (deferred until first prompt) + pending_sessions: Arc>>, } struct ModelStateInfo { @@ -68,6 +77,7 @@ impl ACPAgentImpl { session_manager, session_config_template, model_name, + pending_sessions: Arc::new(Mutex::new(HashMap::new())), playback_path, fast_playback, session_update_tx, @@ -311,55 +321,42 @@ impl acp::Agent for ACPAgentImpl { Self: 'async_trait, 'life0: 'async_trait, { - let session_manager = self.session_manager.clone(); let model_name = self.model_name.clone(); let session_config_template = self.session_config_template.clone(); + let pending_sessions = self.pending_sessions.clone(); Box::pin(async move { tracing::info!("ACP: Creating new session with cwd: {:?}", arguments.cwd); - // Update the agent config to use the provided cwd + let session_id = crate::persistence::generate_session_id(); + let mut session_config = session_config_template.clone(); session_config.init_path = Some(arguments.cwd.clone()); - let session_id = { - let mut manager = session_manager.lock().await; - let session_model_config = SessionModelConfig::new(model_name.clone()); - manager - .create_session_with_config( - None, - Some(session_config), - Some(session_model_config), - ) - .map_err(|e| { - tracing::error!("Failed to create session: {}", e); - to_acp_error(&e) - })? - }; + let selected_model_name = + ACPAgentImpl::compute_model_state(&model_name, Some(model_name.as_str())) + .map(|info| info.selected_model_name.clone()) + .unwrap_or_else(|| model_name.clone()); - tracing::info!("ACP: Created session: {}", session_id); + let session_model_config = SessionModelConfig::new(selected_model_name); - let mut models_state = None; - if let Some(model_info) = - ACPAgentImpl::compute_model_state(&model_name, Some(model_name.as_str())) { - if model_info.selection_changed { - let mut manager = session_manager.lock().await; - let fallback_model_config = - SessionModelConfig::new(model_info.selected_model_name.clone()); - if let Err(err) = - manager.set_session_model_config(&session_id, Some(fallback_model_config)) - { - tracing::error!( - error = ?err, - "ACP: Failed to persist fallback model selection for session {}", - session_id - ); - } - } - models_state = Some(model_info.state); + let mut pending = pending_sessions.lock().await; + pending.insert( + session_id.clone(), + PendingSession { + config: session_config, + model_config: session_model_config, + }, + ); } + tracing::info!("ACP: Created pending session: {}", session_id); + + let models_state = + ACPAgentImpl::compute_model_state(&model_name, Some(model_name.as_str())) + .map(|info| info.state); + Ok(acp::NewSessionResponse::new(session_id).models(models_state)) }) } @@ -484,6 +481,7 @@ impl acp::Agent for ACPAgentImpl { let active_uis = self.active_uis.clone(); let client_capabilities = self.client_capabilities.clone(); let client_connection = get_acp_client_connection(); + let pending_sessions = self.pending_sessions.clone(); Box::pin(async move { tracing::info!( @@ -491,6 +489,30 @@ impl acp::Agent for ACPAgentImpl { arguments.session_id.0 ); + // Materialize pending session on first prompt + if let Some(pending) = pending_sessions + .lock() + .await + .remove(arguments.session_id.0.as_ref()) + { + tracing::info!( + "ACP: Persisting session {} on first prompt", + arguments.session_id.0 + ); + let mut manager = session_manager.lock().await; + manager + .create_session_with_id( + arguments.session_id.0.to_string(), + None, + Some(pending.config), + Some(pending.model_config), + ) + .map_err(|e| { + tracing::error!("Failed to create session: {}", e); + to_acp_error(&e) + })?; + } + let terminal_supported = { let caps = client_capabilities.lock().await; caps.as_ref().map(|caps| caps.terminal).unwrap_or(false) diff --git a/crates/code_assistant/src/session/manager.rs b/crates/code_assistant/src/session/manager.rs index 6f6d3761..502f028c 100644 --- a/crates/code_assistant/src/session/manager.rs +++ b/crates/code_assistant/src/session/manager.rs @@ -90,6 +90,17 @@ impl SessionManager { model_config: Option, ) -> Result { let session_id = generate_session_id(); + self.create_session_with_id(session_id, name, session_config_override, model_config) + } + + /// Create a new session with a specific ID (used for deferred session creation in ACP) + pub fn create_session_with_id( + &mut self, + session_id: String, + name: Option, + session_config_override: Option, + model_config: Option, + ) -> Result { let session_name = name.unwrap_or_default(); // Empty string if no name provided let session = ChatSession::new_empty(