diff --git a/crates/goose/src/acp/server.rs b/crates/goose/src/acp/server.rs index 51a53c914c..75d74d8249 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -148,18 +148,10 @@ async fn ensure_refresh_identity_current( /// called "Session" (the `sessions` DB table) which represents the agent's working /// state: the message list the LLM sees, compaction state, provider binding, etc. /// -/// To bridge these two worlds without rewriting the existing Session model: -/// - **Thread** (`threads` table) = the ACP session. The `sessionId` that ACP clients -/// see is actually a thread ID. Threads own the human-visible message log. -/// - **Session** (`sessions` table) = an internal execution context. A thread may have -/// many sessions over its lifetime (e.g. when the provider or persona changes). -/// Clients never see or manage these directly. -/// -/// The `sessions` HashMap below is keyed by **thread ID** (= ACP session ID). -/// The `internal_session_id` field tracks which goose Session is currently active. +/// The ACP session ID maps directly to a `sessions` row. The `sessions` HashMap +/// below is keyed by session ID. struct GooseAcpSession { agent: AgentHandle, - internal_session_id: String, tool_requests: HashMap, /// For each tool_call_id that belongs to a multi-tool chain (run of /// consecutive ToolRequest blocks within one assistant message), the chain @@ -184,11 +176,11 @@ struct GooseAcpSession { /// LLM summary for the whole run once every step has a recorded ToolResponse. #[derive(Debug, Clone)] struct ToolChain { - /// Assistant message id where every tool request in this chain lives. - /// This is also the row we patch when persisting the chain summary. - message_id: String, /// Tool call ids in document order. Always `len() >= 2`. ids: Vec, + /// The message_id of the assistant message containing these tool calls. + /// Used to persist chain summaries back to the messages table. + message_id: String, } /// Progress stages signalled by the background agent setup task via the watch @@ -233,7 +225,6 @@ pub struct GooseAcpAgent { client_mcp_host_info: OnceCell, config_dir: std::path::PathBuf, session_manager: Arc, - thread_manager: Arc, permission_manager: Arc, goose_mode: GooseMode, disable_session_naming: bool, @@ -248,19 +239,17 @@ fn sid_short(id: &str) -> String { id.chars().take(8).collect() } -fn thread_session_meta( - thread: &crate::session::Thread, -) -> serde_json::Map { +fn session_meta(session: &Session) -> serde_json::Map { let mut meta = serde_json::Map::new(); meta.insert( "messageCount".to_string(), - serde_json::Value::Number(thread.message_count.into()), + serde_json::Value::Number(session.message_count.into()), ); meta.insert( "createdAt".to_string(), - serde_json::Value::String(thread.created_at.to_rfc3339()), + serde_json::Value::String(session.created_at.to_rfc3339()), ); - if let Some(ref archived_at) = thread.archived_at { + if let Some(ref archived_at) = session.archived_at { meta.insert( "archivedAt".to_string(), serde_json::Value::String(archived_at.to_rfc3339()), @@ -268,30 +257,25 @@ fn thread_session_meta( } meta.insert( "userSetName".to_string(), - serde_json::Value::Bool(thread.user_set_name), + serde_json::Value::Bool(session.user_set_name), ); - if let Some(ref pid) = thread.metadata.project_id { + + if let Some(ref pid) = session.project_id { meta.insert( "projectId".to_string(), serde_json::Value::String(pid.clone()), ); } - if let Some(ref provider_id) = thread.metadata.provider_id { + if let Some(ref provider) = session.provider_name { meta.insert( "providerId".to_string(), - serde_json::Value::String(provider_id.clone()), + serde_json::Value::String(provider.clone()), ); } - if let Some(ref model_id) = thread.metadata.model_id { + if let Some(ref mc) = session.model_config { meta.insert( "modelId".to_string(), - serde_json::Value::String(model_id.clone()), - ); - } - if let Some(ref persona_id) = thread.metadata.persona_id { - meta.insert( - "personaId".to_string(), - serde_json::Value::String(persona_id.clone()), + serde_json::Value::String(mc.model_name.clone()), ); } meta @@ -303,21 +287,27 @@ fn spawn_session_name_update_notifier( let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel::(); tokio::spawn(async move { while let Some(update) = rx.recv().await { - let thread = update.thread; - let thread_id = thread.id.clone(); - let meta = thread_session_meta(&thread); + let mut meta = serde_json::Map::new(); + meta.insert( + "messageCount".to_string(), + serde_json::Value::Number(update.message_count.into()), + ); + meta.insert( + "userSetName".to_string(), + serde_json::Value::Bool(update.user_set_name), + ); let notification = SessionNotification::new( - SessionId::new(thread_id.clone()), + SessionId::new(update.session_id.clone()), SessionUpdate::SessionInfoUpdate( SessionInfoUpdate::new() - .title(thread.name) - .updated_at(thread.updated_at.to_rfc3339()) + .title(update.name) + .updated_at(update.updated_at.to_rfc3339()) .meta(meta), ), ); if let Err(error) = cx.send_notification(notification) { warn!( - thread_id = %thread_id, + session_id = %update.session_id, error = %error, "Failed to send generated session name update" ); @@ -732,7 +722,7 @@ fn extract_tool_chains( /// If `buffer` holds a multi-tool run (≥ 2 tool requests), (re)register a /// [`ToolChain`] in `chain_membership` anchored on the **first** tool's -/// message_id (the row [`ThreadManager::update_tool_request_meta`] will patch +/// message_id (the row [`SessionManager::update_tool_request_meta`] will patch /// when persisting the LLM-generated summary). Does **not** clear the buffer /// — chains can grow as more tools arrive (sequential tool use), so callers /// keep accumulating and re-registering with the larger set of ids. @@ -750,11 +740,11 @@ fn extend_chain_membership( chain_membership: &mut HashMap>, ) { if buffer.len() >= 2 { - let anchor_message_id = buffer[0].1.clone(); let ids: Vec = buffer.iter().map(|(id, _)| id.clone()).collect(); + let message_id = buffer[0].1.clone(); let chain = Arc::new(ToolChain { - message_id: anchor_message_id, ids: ids.clone(), + message_id, }); for id in ids { chain_membership.insert(id, chain.clone()); @@ -1063,9 +1053,6 @@ impl GooseAcpAgent { let _ = storage_clone.pool().await; }); - let thread_manager = Arc::new(crate::session::ThreadManager::new( - session_manager.storage().clone(), - )); let permission_manager = Arc::new(PermissionManager::new(config_dir.clone())); let provider_inventory = ProviderInventoryService::new(session_manager.storage().clone()); @@ -1078,7 +1065,6 @@ impl GooseAcpAgent { client_mcp_host_info: OnceCell::new(), config_dir, session_manager, - thread_manager, permission_manager, goose_mode, disable_session_naming, @@ -1289,8 +1275,8 @@ impl GooseAcpAgent { } = req; let goose_mode = goose_session.goose_mode; - let internal_session_id = goose_session.id.clone(); - let agent_session_id = SessionId::new(internal_session_id.clone()); + let setup_session_id = goose_session.id.clone(); + let agent_session_id = SessionId::new(setup_session_id.clone()); let sid = sid_short(session_id.0.as_ref()); let cx = cx.clone(); @@ -1365,7 +1351,7 @@ impl GooseAcpAgent { .map_err(|e| e.to_string())?; agent - .update_goose_mode(goose_mode, &internal_session_id) + .update_goose_mode(goose_mode, &setup_session_id) .await .map_err(|e| e.to_string())?; @@ -1473,7 +1459,7 @@ impl GooseAcpAgent { .await; } - GooseAcpAgent::add_mcp_extensions(&agent, mcp_servers, &internal_session_id) + GooseAcpAgent::add_mcp_extensions(&agent, mcp_servers, &setup_session_id) .await .map_err(|e| e.to_string())?; @@ -1590,7 +1576,7 @@ impl GooseAcpAgent { &self, content_item: &MessageContent, session_id: &SessionId, - thread_id: &str, + session_id_str: &str, message_id: Option<&str>, agent: &Arc, session: &mut GooseAcpSession, @@ -1609,7 +1595,7 @@ impl GooseAcpAgent { self.handle_tool_request( tool_request, session_id, - thread_id, + session_id_str, message_id, session, cx, @@ -1620,7 +1606,7 @@ impl GooseAcpAgent { self.handle_tool_response( tool_response, session_id, - thread_id, + session_id_str, message_id, session, cx, @@ -1663,7 +1649,7 @@ impl GooseAcpAgent { &self, tool_request: &crate::conversation::message::ToolRequest, session_id: &SessionId, - thread_id: &str, + session_id_for_persist: &str, message_id: Option<&str>, session: &mut GooseAcpSession, cx: &ConnectionTo, @@ -1692,6 +1678,9 @@ impl GooseAcpAgent { let name = tool_call.name.to_string(); let identity_meta = pending_tool_call.identity_meta.clone(); let fallback_title = pending_tool_call.fallback_title.clone(); + let session_id_for_persist = session_id_for_persist.to_string(); + let message_id_for_persist = message_id.map(|s| s.to_string()); + let session_manager = self.session_manager.clone(); let args_json = tool_call .arguments .as_ref() @@ -1705,10 +1694,6 @@ impl GooseAcpAgent { }) .unwrap_or_default(); - let thread_id_for_persist = thread_id.to_string(); - let message_id_for_persist = message_id.map(|s| s.to_string()); - let thread_manager = self.thread_manager.clone(); - tokio::spawn(async move { let (title, from_llm) = match agent.provider().await { Ok(provider) => { @@ -1794,17 +1779,15 @@ impl GooseAcpAgent { // Best-effort persistence: only persist the LLM-generated title // (not the deterministic fallback) so reload uses fallback_title - // for older or failed cases just like today. Surface persist - // errors at warn level so the "occasional bad replay" symptom is - // diagnosable from logs alone. + // for older or failed cases just like today. if from_llm { if let Some(msg_id) = message_id_for_persist { let patch = serde_json::json!({ crate::conversation::message::TOOL_META_TITLE_KEY: title, }); - if let Err(e) = thread_manager + if let Err(e) = session_manager .update_tool_request_meta( - &thread_id_for_persist, + &session_id_for_persist, &msg_id, &request_id, patch, @@ -1831,7 +1814,7 @@ impl GooseAcpAgent { &self, tool_response: &crate::conversation::message::ToolResponse, session_id: &SessionId, - thread_id: &str, + session_id_str: &str, message_id: Option<&str>, session: &mut GooseAcpSession, cx: &ConnectionTo, @@ -1876,7 +1859,7 @@ impl GooseAcpAgent { // Chain summarization: when this response completes a multi-tool // chain, fire one LLM summary covering the run. session.responded_tool_ids.insert(tool_response.id.clone()); - self.maybe_summarize_chain(&tool_response.id, session_id, thread_id, session, cx); + self.maybe_summarize_chain(&tool_response.id, session_id, session_id_str, session, cx); let _ = message_id; Ok(()) @@ -1891,7 +1874,7 @@ impl GooseAcpAgent { &self, tool_call_id: &str, session_id: &SessionId, - thread_id: &str, + _session_id_str: &str, session: &mut GooseAcpSession, cx: &ConnectionTo, ) { @@ -1974,10 +1957,9 @@ impl GooseAcpAgent { .and_then(tool_call_identity_meta); let sid = session_id.clone(); - let thread_id_for_persist = thread_id.to_string(); let chain_for_task = chain.clone(); let cx = cx.clone(); - let thread_manager = self.thread_manager.clone(); + let session_manager = self.session_manager.clone(); let first_id = first_id.clone(); tokio::spawn(async move { @@ -2065,13 +2047,8 @@ impl GooseAcpAgent { "count": count, }, }); - if let Err(e) = thread_manager - .update_tool_request_meta( - &thread_id_for_persist, - &chain_for_task.message_id, - &first_id, - patch, - ) + if let Err(e) = session_manager + .update_tool_request_meta(&sid.0, &chain_for_task.message_id, &first_id, patch) .await { warn!( @@ -2338,54 +2315,57 @@ impl GooseAcpAgent { .and_then(|v| v.as_str()) .map(|s| s.to_string()); - let persona_id = args + // When _meta.client is set, the session is created by a known client + // (e.g. "goose" for the desktop app) and treated as a User session. + // Without it, sessions default to Acp for programmatic ACP clients. + let session_type = match args .meta .as_ref() - .and_then(|m| m.get("personaId")) + .and_then(|m| m.get("client")) .and_then(|v| v.as_str()) - .map(|s| s.to_string()); - - // Create the Thread — this IS the ACP session from the client's perspective. - let thread_metadata = crate::session::ThreadMetadata { - provider_id: requested_provider.clone(), - project_id, - persona_id, - mode: Some(self.goose_mode.to_string()), - ..Default::default() + { + Some(_) => SessionType::User, + None => SessionType::Acp, }; + let t0 = std::time::Instant::now(); - let thread = self - .thread_manager - .create_thread( - None, - Some(thread_metadata), - Some(args.cwd.display().to_string()), + let goose_session = self + .session_manager + .create_session( + args.cwd.clone(), + "New Chat".to_string(), + session_type, + self.goose_mode, ) .await - .internal_err_ctx("Failed to create thread")?; - let thread_id = thread.id.clone(); - let sid = sid_short(&thread_id); - debug!(target: "perf", sid = %sid, ms = t0.elapsed().as_millis() as u64, "perf: new_session create_thread"); + .internal_err_ctx("Failed to create session")?; + + let mut builder = self.session_manager.update(&goose_session.id); + if let Some(ref provider) = requested_provider { + builder = builder.provider_name(provider); + } + if let Some(pid) = project_id { + builder = builder.project_id(Some(pid)); + } + builder + .apply() + .await + .internal_err_ctx("Failed to update session")?; - // Create the first internal Session linked to this thread. - let t1 = std::time::Instant::now(); let goose_session = self - .create_internal_session( - &thread_id, - args.cwd.clone(), - requested_provider.as_deref(), - None, - ) - .await?; - debug!(target: "perf", sid = %sid, ms = t1.elapsed().as_millis() as u64, "perf: new_session create_internal_session"); + .session_manager + .get_session(&goose_session.id, false) + .await + .internal_err_ctx("Failed to reload session")?; - let internal_session_id = goose_session.id.clone(); + let session_id_str = goose_session.id.clone(); + let sid = sid_short(&session_id_str); + debug!(target: "perf", sid = %sid, ms = t0.elapsed().as_millis() as u64, "perf: new_session create_session"); let (agent_tx, agent_rx) = tokio::sync::watch::channel::(None); - let session = GooseAcpSession { + let acp_session = GooseAcpSession { agent: AgentHandle::Loading(agent_rx), - internal_session_id: internal_session_id.clone(), tool_requests: HashMap::new(), chain_membership: HashMap::new(), responded_tool_ids: HashSet::new(), @@ -2396,18 +2376,16 @@ impl GooseAcpAgent { self.sessions .lock() .await - .insert(thread_id.clone(), session); + .insert(session_id_str.clone(), acp_session); let mode_state = build_mode_state(self.goose_mode)?; - // Resolve provider + model from config so we can include the current - // model in the response without waiting for the full agent setup. let resolved = resolve_provider_and_model(&self.config_dir, &goose_session).await; let initial_usage_update = resolved .as_ref() .ok() .map(|(_, mc)| build_usage_update(&goose_session, mc.context_limit())); - let session_id = SessionId::new(thread_id.clone()); + let acp_session_id = SessionId::new(session_id_str); let (model_state, config_options, prebuilt_provider) = self .prepare_session_init_config(&resolved, &mode_state, &goose_session) .await; @@ -2416,7 +2394,7 @@ impl GooseAcpAgent { cx, agent_tx, AgentSetupRequest { - session_id: session_id.clone(), + session_id: acp_session_id.clone(), goose_session, mcp_servers: args.mcp_servers, resolved_provider: resolved.as_ref().ok().cloned(), @@ -2424,7 +2402,7 @@ impl GooseAcpAgent { }, ); - let mut response = NewSessionResponse::new(session_id.clone()).modes(mode_state); + let mut response = NewSessionResponse::new(acp_session_id.clone()).modes(mode_state); if let Some(ms) = model_state { response = response.models(ms); } @@ -2433,7 +2411,7 @@ impl GooseAcpAgent { } if let Some(usage_update) = initial_usage_update { cx.send_notification(SessionNotification::new( - session_id, + acp_session_id, SessionUpdate::UsageUpdate(usage_update), ))?; } @@ -2446,62 +2424,21 @@ impl GooseAcpAgent { Ok(response) } - /// Create a new internal goose Session linked to a thread. - /// This is the agent's working state — invisible to ACP clients. - async fn create_internal_session( - &self, - thread_id: &str, - cwd: std::path::PathBuf, - provider_name: Option<&str>, - model_name: Option<&str>, - ) -> Result { - let goose_session = self - .session_manager - .create_session( - cwd, - "ACP Session".to_string(), - SessionType::Acp, - self.goose_mode, - ) - .await - .internal_err_ctx("Failed to create session")?; - - let mut builder = self.session_manager.update(&goose_session.id); - builder = builder.thread_id(Some(thread_id.to_string())); - if let Some(provider) = provider_name { - builder = builder.provider_name(provider); - } - if let Some(model) = model_name { - if let Ok(mc) = crate::model::ModelConfig::new(model) { - builder = builder.model_config(mc); - } - } - builder - .apply() - .await - .internal_err_ctx("Failed to link session to thread")?; - - self.session_manager - .get_session(&goose_session.id, false) - .await - .internal_err_ctx("Failed to reload session") - } - /// Look up the session and return the agent if already ready, or the watch /// receiver if still loading. Optionally sets a cancellation token on the /// session (needed by `on_prompt`). async fn get_agent_or_receiver( &self, - thread_id: &str, + session_id: &str, cancel_token: Option, ) -> Result< Either, tokio::sync::watch::Receiver>, agent_client_protocol::Error, > { let mut sessions = self.sessions.lock().await; - let session = sessions.get_mut(thread_id).ok_or_else(|| { - agent_client_protocol::Error::resource_not_found(Some(thread_id.to_string())) - .data(format!("Session not found: {}", thread_id)) + let session = sessions.get_mut(session_id).ok_or_else(|| { + agent_client_protocol::Error::resource_not_found(Some(session_id.to_string())) + .data(format!("Session not found: {}", session_id)) })?; if let Some(token) = cancel_token { session.cancel_token = Some(token); @@ -2516,10 +2453,10 @@ impl GooseAcpAgent { /// Most callers (e.g. `on_prompt`, `on_get_tools`) should use this. async fn get_session_agent( &self, - thread_id: &str, + session_id: &str, cancel_token: Option, ) -> Result, agent_client_protocol::Error> { - let mut rx = match self.get_agent_or_receiver(thread_id, cancel_token).await? { + let mut rx = match self.get_agent_or_receiver(session_id, cancel_token).await? { Either::Left(agent) => return Ok(agent), Either::Right(rx) => rx, }; @@ -2549,9 +2486,9 @@ impl GooseAcpAgent { /// the provider (e.g. `update_provider`, `set_model`, `build_config_update`). async fn get_session_agent_provider_ready( &self, - thread_id: &str, + session_id: &str, ) -> Result, agent_client_protocol::Error> { - let mut rx = match self.get_agent_or_receiver(thread_id, None).await? { + let mut rx = match self.get_agent_or_receiver(session_id, None).await? { Either::Left(agent) => return Ok(agent), Either::Right(rx) => rx, }; @@ -2572,7 +2509,7 @@ impl GooseAcpAgent { async fn add_mcp_extensions( agent: &Arc, mcp_servers: Vec, - internal_session_id: &str, + session_id: &str, ) -> Result<(), agent_client_protocol::Error> { let mut configs = Vec::with_capacity(mcp_servers.len()); for mcp_server in mcp_servers { @@ -2590,7 +2527,7 @@ impl GooseAcpAgent { } let results = agent - .add_extensions_bulk(configs, internal_session_id) + .add_extensions_bulk(configs, session_id) .await .internal_err()?; for result in &results { @@ -2612,64 +2549,41 @@ impl GooseAcpAgent { ) -> Result { debug!(?args, "load session request"); - // The ACP session_id IS the thread ID. - let thread_id = args.session_id.0.to_string(); - let sid = sid_short(&thread_id); + let session_id = args.session_id.0.to_string(); + let sid = sid_short(&session_id); let t_start = std::time::Instant::now(); let t0 = std::time::Instant::now(); - let thread = self - .thread_manager - .get_thread(&thread_id) - .await - .map_err(|_| { - agent_client_protocol::Error::resource_not_found(Some(thread_id.clone())) - .data(format!("Session not found: {}", thread_id)) - })?; - debug!(target: "perf", sid = %sid, ms = t0.elapsed().as_millis() as u64, "perf: load_session get_thread"); - - // Reuse the thread's current internal session so the agent retains - // conversation context (compaction state, full message history, etc.). - // The internal session is the source of truth for provider/mode. - let internal_session_id = thread.current_session_id.clone().ok_or_else(|| { - agent_client_protocol::Error::internal_error() - .data(format!("Thread {} has no internal session", thread_id)) - })?; - let t1 = std::time::Instant::now(); let goose_session = self .session_manager - .get_session(&internal_session_id, false) + .get_session(&session_id, true) .await - .internal_err_ctx("Failed to load internal session")?; - debug!(target: "perf", sid = %sid, ms = t1.elapsed().as_millis() as u64, "perf: load_session get_session"); + .map_err(|_| { + agent_client_protocol::Error::resource_not_found(Some(session_id.clone())) + .data(format!("Session not found: {}", session_id)) + })?; + debug!(target: "perf", sid = %sid, ms = t0.elapsed().as_millis() as u64, "perf: load_session get_session"); let loaded_mode = goose_session.goose_mode; - // ── REPLAY MESSAGES FIRST ── - // Stream the thread's human-visible message history back to the client - // immediately, before the slow agent/provider/extension setup. The - // replay only needs the thread_manager (SQLite reads) so the UI gets - // messages while the agent is still booting. - let t2 = std::time::Instant::now(); - let thread_messages = self - .thread_manager - .list_messages(&thread_id) - .await - .internal_err_ctx("Failed to load thread messages")?; + // ── REPLAY MESSAGES ── + // Stream user-visible messages back to the client so the chat view + // populates immediately, before the slow agent/provider/extension setup. + let messages = goose_session + .conversation + .as_ref() + .map(|c| c.messages().to_vec()) + .unwrap_or_default(); debug!( target: "perf", sid = %sid, - ms = t2.elapsed().as_millis() as u64, - messages = thread_messages.len(), - "perf: load_session list_messages" + messages = messages.len(), + "perf: load_session messages loaded" ); - // Lightweight tool_requests map for the replay loop — we only need it - // so that handle_tool_response can extract file locations from the - // matching request. No GooseAcpSession required. let mut replay_tool_requests = HashMap::::new(); - for message in &thread_messages { + for message in &messages { if !message.metadata.user_visible { continue; } @@ -2798,25 +2712,19 @@ impl GooseAcpAgent { } } - // ── Lightweight DB updates (fast) ── + // Update working directory. self.session_manager - .update(&internal_session_id) + .update(&session_id) .working_dir(args.cwd.clone()) .apply() .await .internal_err_ctx("Failed to update session working directory")?; - self.thread_manager - .update_working_dir(&thread_id, &args.cwd.display().to_string()) - .await - .internal_err_ctx("Failed to update thread working directory")?; - - // ── Register the session immediately with a Loading handle ── + // Register the session with a Loading handle. let (agent_tx, agent_rx) = tokio::sync::watch::channel::(None); - let session = GooseAcpSession { + let acp_session = GooseAcpSession { agent: AgentHandle::Loading(agent_rx), - internal_session_id: internal_session_id.clone(), tool_requests: replay_tool_requests, chain_membership: HashMap::new(), responded_tool_ids: HashSet::new(), @@ -2827,7 +2735,7 @@ impl GooseAcpAgent { self.sessions .lock() .await - .insert(thread_id.clone(), session); + .insert(session_id.clone(), acp_session); let mode_state = build_mode_state(loaded_mode)?; @@ -2886,42 +2794,19 @@ impl GooseAcpAgent { args: PromptRequest, ) -> Result { // The ACP session_id IS the thread ID. - let thread_id = args.session_id.0.to_string(); - let sid = sid_short(&thread_id); + let session_id = args.session_id.0.to_string(); + let sid = sid_short(&session_id); let t_start = std::time::Instant::now(); - // Update persona_id on the thread if the client sent one in _meta. - let prompt_persona_id = args - .meta - .as_ref() - .and_then(|m| m.get("personaId")) - .and_then(|v| v.as_str()) - .map(|s| s.to_string()); - if let Some(ref pid) = prompt_persona_id { - let pid = pid.clone(); - self.update_thread_metadata(&thread_id, move |meta| { - meta.persona_id = Some(pid); - }) - .await?; - } - let cancel_token = CancellationToken::new(); - let internal_session_id = self.internal_session_id(&thread_id).await?; - let agent = self - .get_session_agent(&thread_id, Some(cancel_token.clone())) + .get_session_agent(&session_id, Some(cancel_token.clone())) .await?; let user_message = Self::convert_acp_prompt_to_message(&args.prompt); - // Persist user message (may contain assistant-only annotated blocks) - self.thread_manager - .append_message(&thread_id, Some(&internal_session_id), &user_message) - .await - .internal_err_ctx("Failed to persist message")?; - let session_config = SessionConfig { - id: internal_session_id.clone(), + id: session_id.clone(), schedule_id: None, max_turns: None, retry_config: None, @@ -2965,20 +2850,16 @@ impl GooseAcpAgent { match event { Ok(crate::agents::AgentEvent::Message(message)) => { - let stored_message = self - .thread_manager - .append_message(&thread_id, Some(&internal_session_id), &message) - .await - .internal_err_ctx("Failed to persist message")?; - let stored_message_id = stored_message.id.clone(); + // Agent persists messages via session_manager.add_message() internally. + let stored_message_id = message.id.clone(); let mut sessions = self.sessions.lock().await; - let session = sessions.get_mut(&thread_id).ok_or_else(|| { + let session = sessions.get_mut(&session_id).ok_or_else(|| { agent_client_protocol::Error::invalid_params() - .data(format!("Session not found: {}", thread_id)) + .data(format!("Session not found: {}", session_id)) })?; - for content_item in &stored_message.content { + for content_item in &message.content { match content_item { MessageContent::ToolRequest(tr) => { if let Some(msg_id) = stored_message_id.as_deref() { @@ -3009,7 +2890,7 @@ impl GooseAcpAgent { self.handle_message_content( content_item, &args.session_id, - &thread_id, + &session_id, stored_message_id.as_deref(), &agent, session, @@ -3028,7 +2909,7 @@ impl GooseAcpAgent { { let mut sessions = self.sessions.lock().await; - if let Some(session) = sessions.get_mut(&thread_id) { + if let Some(session) = sessions.get_mut(&session_id) { // Final safety net: in case the stream ended without any // chain-breaking content, make sure a multi-tool buffer is // registered. (Eager registration during the loop usually @@ -3040,7 +2921,7 @@ impl GooseAcpAgent { let session = self .session_manager - .get_session(&internal_session_id, false) + .get_session(&session_id, false) .await .internal_err_ctx("Failed to load session")?; let provider = agent @@ -3081,16 +2962,16 @@ impl GooseAcpAgent { ) -> Result<(), agent_client_protocol::Error> { debug!(?args, "cancel request"); - let thread_id = args.session_id.0.to_string(); + let session_id = args.session_id.0.to_string(); let mut sessions = self.sessions.lock().await; - if let Some(session) = sessions.get_mut(&thread_id) { + if let Some(session) = sessions.get_mut(&session_id) { if let Some(ref token) = session.cancel_token { - info!(thread_id = %thread_id, "prompt cancelled"); + info!(session_id = %session_id, "prompt cancelled"); token.cancel(); } } else { - warn!(thread_id = %thread_id, "cancel request for unknown session"); + warn!(session_id = %session_id, "cancel request for unknown session"); } Ok(()) @@ -3098,19 +2979,18 @@ impl GooseAcpAgent { async fn on_set_model( &self, - thread_id: &str, + session_id: &str, model_id: &str, ) -> Result { - let internal_id = self.internal_session_id(thread_id).await?; let config = self.config()?; - let agent = self.get_session_agent_provider_ready(thread_id).await?; + let agent = self.get_session_agent_provider_ready(session_id).await?; let current_provider = agent .provider() .await .internal_err_ctx("Failed to get provider")?; let provider_name = current_provider.get_name().to_string(); let extensions = - EnabledExtensionsState::for_session(&self.session_manager, &internal_id, &config).await; + EnabledExtensionsState::for_session(&self.session_manager, session_id, &config).await; let model_config = crate::model::ModelConfig::new(model_id) .invalid_params_err_ctx("Invalid model config")? .with_canonical_limits(&provider_name); @@ -3119,60 +2999,28 @@ impl GooseAcpAgent { .await .internal_err_ctx("Failed to create provider")?; agent - .update_provider(provider, &internal_id) + .update_provider(provider, session_id) .await .internal_err_ctx("Failed to update provider")?; let mode = agent.goose_mode().await; agent - .update_goose_mode(mode, &internal_id) + .update_goose_mode(mode, session_id) .await .internal_err_ctx("Failed to propagate mode")?; - let model_id_owned = model_id.to_string(); - self.update_thread_metadata(thread_id, move |meta| { - meta.model_id = Some(model_id_owned); - }) - .await?; + // model_config is already updated on the session by the agent's update_provider call. Ok(SetSessionModelResponse::new()) } - async fn internal_session_id( - &self, - thread_id: &str, - ) -> Result { - self.sessions - .lock() - .await - .get(thread_id) - .map(|s| s.internal_session_id.clone()) - .ok_or_else(|| { - agent_client_protocol::Error::resource_not_found(Some(thread_id.to_string())) - .data(format!("Session not found: {}", thread_id)) - }) - } - - pub(super) async fn update_thread_metadata( - &self, - thread_id: &str, - f: impl FnOnce(&mut crate::session::ThreadMetadata), - ) -> Result<(), agent_client_protocol::Error> { - self.thread_manager - .update_metadata(thread_id, f) - .await - .internal_err()?; - Ok(()) - } - async fn build_config_update( &self, - thread_id: &SessionId, + session_id: &SessionId, ) -> Result<(SessionNotification, Vec), agent_client_protocol::Error> { - let internal_id = self.internal_session_id(&thread_id.0).await?; let session = self .session_manager - .get_session(&internal_id, false) + .get_session(&session_id.0, false) .await .internal_err()?; - let agent = self.get_session_agent_provider_ready(&thread_id.0).await?; + let agent = self.get_session_agent_provider_ready(&session_id.0).await?; let provider = agent .provider() .await @@ -3199,7 +3047,7 @@ impl GooseAcpAgent { provider_options, ); let notification = SessionNotification::new( - thread_id.clone(), + session_id.clone(), SessionUpdate::ConfigOptionUpdate(ConfigOptionUpdate::new(config_options.clone())), ); Ok((notification, config_options)) @@ -3207,41 +3055,35 @@ impl GooseAcpAgent { async fn on_set_mode( &self, - thread_id: &str, + session_id: &str, mode_id: &str, ) -> Result { - let internal_id = self.internal_session_id(thread_id).await?; let mode = mode_id.parse::().map_err(|_| { agent_client_protocol::Error::invalid_params() .data(format!("Invalid mode: {}", mode_id)) })?; - let agent = self.get_session_agent_provider_ready(thread_id).await?; + let agent = self.get_session_agent_provider_ready(session_id).await?; agent - .update_goose_mode(mode, &internal_id) + .update_goose_mode(mode, session_id) .await .internal_err_ctx("Failed to update mode")?; - let mode_id = mode_id.to_string(); - self.update_thread_metadata(thread_id, move |meta| { - meta.mode = Some(mode_id); - }) - .await?; + // goose_mode is already updated on the session above. Ok(SetSessionModeResponse::new()) } async fn update_provider( &self, - thread_id: &str, + session_id: &str, provider_name: &str, model_name: Option<&str>, context_limit: Option, request_params: Option>, ) -> Result<(), agent_client_protocol::Error> { - let internal_id = self.internal_session_id(thread_id).await?; let config = self.config()?; - let agent = self.get_session_agent_provider_ready(thread_id).await?; + let agent = self.get_session_agent_provider_ready(session_id).await?; let current_provider = agent .provider() .await @@ -3278,18 +3120,18 @@ impl GooseAcpAgent { .with_request_params(request_params); let extensions = - EnabledExtensionsState::for_session(&self.session_manager, &internal_id, &config).await; + EnabledExtensionsState::for_session(&self.session_manager, session_id, &config).await; let new_provider = self .create_provider(&resolved_provider_name, model_config, extensions) .await .internal_err_ctx("Failed to create provider")?; agent - .update_provider(new_provider, &internal_id) + .update_provider(new_provider, session_id) .await .internal_err_ctx("Failed to update provider")?; let mode = agent.goose_mode().await; agent - .update_goose_mode(mode, &internal_id) + .update_goose_mode(mode, session_id) .await .internal_err_ctx("Failed to propagate mode")?; let provider = agent @@ -3297,17 +3139,12 @@ impl GooseAcpAgent { .await .internal_err_ctx("Failed to get provider")?; - let provider_name_owned = provider_name.to_string(); - self.update_thread_metadata(thread_id, move |meta| { - meta.provider_id = Some(provider_name_owned); - meta.model_id = None; - }) - .await?; + // provider_name is already updated on the session by the agent's update_provider call. if use_default_provider { let update = self .session_manager - .update(&internal_id) + .update(session_id) .provider_name(DEFAULT_PROVIDER_ID); if has_default_overrides { update @@ -3327,24 +3164,20 @@ impl GooseAcpAgent { } async fn on_list_sessions(&self) -> Result { - // Return threads (= ACP sessions), not internal goose sessions. - let threads = self - .thread_manager - .list_threads(false) + // ACP clients see their own (Acp) sessions plus legacy User/Scheduled ones. + let sessions = self + .session_manager + .list_sessions_by_types(&[SessionType::User, SessionType::Scheduled, SessionType::Acp]) .await .internal_err()?; - let session_infos: Vec = threads + let session_infos: Vec = sessions .into_iter() - .map(|t| { - let cwd = t - .working_dir - .as_deref() - .map(std::path::PathBuf::from) - .unwrap_or_default(); - let meta = thread_session_meta(&t); - SessionInfo::new(SessionId::new(t.id), cwd) - .title(t.name) - .updated_at(t.updated_at.to_rfc3339()) + .filter(|s| s.message_count > 0) + .map(|s| { + let meta = session_meta(&s); + SessionInfo::new(SessionId::new(s.id), s.working_dir) + .title(s.name) + .updated_at(s.updated_at.to_rfc3339()) .meta(meta) }) .collect(); @@ -3356,28 +3189,33 @@ impl GooseAcpAgent { cx: &ConnectionTo, args: ForkSessionRequest, ) -> Result { - let source_thread_id = &*args.session_id.0; + let source_session_id = &*args.session_id.0; - // Fork the thread (copies metadata + messages). - let new_thread = self - .thread_manager - .fork_thread(source_thread_id) + let new_session = self + .session_manager + .copy_session(source_session_id, "Fork".to_string()) + .await + .internal_err()?; + let new_session_id = new_session.id.clone(); + + // Update working dir for the fork. + self.session_manager + .update(&new_session_id) + .working_dir(args.cwd.clone()) + .apply() .await .internal_err()?; - let new_thread_id = new_thread.id.clone(); - // Create an internal session for the new thread. let goose_session = self - .create_internal_session(&new_thread_id, args.cwd, None, None) - .await?; - - let internal_session_id = goose_session.id.clone(); + .session_manager + .get_session(&new_session_id, false) + .await + .internal_err()?; let (agent_tx, agent_rx) = tokio::sync::watch::channel::(None); - let session = GooseAcpSession { + let acp_session = GooseAcpSession { agent: AgentHandle::Loading(agent_rx), - internal_session_id: internal_session_id.clone(), tool_requests: HashMap::new(), chain_membership: HashMap::new(), responded_tool_ids: HashSet::new(), @@ -3388,7 +3226,7 @@ impl GooseAcpAgent { self.sessions .lock() .await - .insert(new_thread_id.clone(), session); + .insert(new_session_id.clone(), acp_session); let mode_state = build_mode_state(self.goose_mode)?; let resolved = resolve_provider_and_model(&self.config_dir, &goose_session).await; @@ -3400,7 +3238,7 @@ impl GooseAcpAgent { cx, agent_tx, AgentSetupRequest { - session_id: SessionId::new(new_thread_id.clone()), + session_id: SessionId::new(new_session_id.clone()), goose_session, mcp_servers: args.mcp_servers, resolved_provider: resolved.ok(), @@ -3408,9 +3246,9 @@ impl GooseAcpAgent { }, ); - let meta = thread_session_meta(&new_thread); + let meta = session_meta(&new_session); - let mut response = ForkSessionResponse::new(SessionId::new(new_thread_id)) + let mut response = ForkSessionResponse::new(SessionId::new(new_session_id)) .modes(mode_state) .meta(meta); if let Some(ms) = model_state { @@ -3424,17 +3262,16 @@ impl GooseAcpAgent { async fn on_close_session( &self, - thread_id: &str, + session_id: &str, ) -> Result { - // Tear down the in-memory agent. The thread persists for later session/load. let mut sessions = self.sessions.lock().await; - if let Some(session) = sessions.get(thread_id) { + if let Some(session) = sessions.get(session_id) { if let Some(ref token) = session.cancel_token { token.cancel(); } } - sessions.remove(thread_id); - info!(thread_id = %thread_id, "ACP session closed (thread preserved)"); + sessions.remove(session_id); + info!(session_id = %session_id, "ACP session closed"); Ok(CloseSessionResponse::new()) } } @@ -3774,7 +3611,6 @@ print(\"hello, world\") Arc::ptr_eq(chain_a, chain_b) && Arc::ptr_eq(chain_b, chain_c), "every id in the run must point at the same ToolChain Arc", ); - assert_eq!(chain_a.message_id, "row_first"); assert_eq!( chain_a.ids, vec!["a".to_string(), "b".to_string(), "c".to_string()], @@ -3799,7 +3635,6 @@ print(\"hello, world\") let chain = membership .get("toolu_bdrk_1") .expect("first tool registered"); - assert_eq!(chain.message_id, "row_for_tool_1"); assert_eq!( chain.ids, vec!["toolu_bdrk_1".to_string(), "toolu_bdrk_2".to_string()], @@ -3827,7 +3662,6 @@ print(\"hello, world\") let chain_b = membership.get("b").expect("b present"); let chain_c = membership.get("c").expect("c present"); assert!(Arc::ptr_eq(chain_a, chain_b) && Arc::ptr_eq(chain_b, chain_c)); - assert_eq!(chain_a.message_id, "row_1"); assert_eq!( chain_a.ids, vec!["a".to_string(), "b".to_string(), "c".to_string()], @@ -4316,7 +4150,8 @@ print(\"hello, world\") provider_name: None, model_config: None, goose_mode: GooseMode::default(), - thread_id: None, + archived_at: None, + project_id: None, } } diff --git a/crates/goose/src/acp/server/extensions.rs b/crates/goose/src/acp/server/extensions.rs index ff8ca3082a..c92dcb8635 100644 --- a/crates/goose/src/acp/server/extensions.rs +++ b/crates/goose/src/acp/server/extensions.rs @@ -5,13 +5,13 @@ impl GooseAcpAgent { &self, req: AddExtensionRequest, ) -> Result { - let internal_id = self.internal_session_id(&req.session_id).await?; + let session_id = &req.session_id; let config: ExtensionConfig = serde_json::from_value(req.config).map_err(|e| { agent_client_protocol::Error::invalid_params().data(format!("bad config: {e}")) })?; let agent = self.get_session_agent(&req.session_id, None).await?; agent - .add_extension(config, &internal_id) + .add_extension(config, session_id) .await .internal_err()?; Ok(EmptyResponse {}) @@ -21,10 +21,10 @@ impl GooseAcpAgent { &self, req: RemoveExtensionRequest, ) -> Result { - let internal_id = self.internal_session_id(&req.session_id).await?; + let session_id = &req.session_id; let agent = self.get_session_agent(&req.session_id, None).await?; agent - .remove_extension(&req.name, &internal_id) + .remove_extension(&req.name, session_id) .await .internal_err()?; Ok(EmptyResponse {}) @@ -114,10 +114,10 @@ impl GooseAcpAgent { &self, req: GetSessionExtensionsRequest, ) -> Result { - let internal_id = self.internal_session_id(&req.session_id).await?; + let session_id = &req.session_id; let session = self .session_manager - .get_session(&internal_id, false) + .get_session(session_id, false) .await .internal_err()?; diff --git a/crates/goose/src/acp/server/resources.rs b/crates/goose/src/acp/server/resources.rs index 859024c4ac..ba2f07bff5 100644 --- a/crates/goose/src/acp/server/resources.rs +++ b/crates/goose/src/acp/server/resources.rs @@ -5,12 +5,12 @@ impl GooseAcpAgent { &self, req: ReadResourceRequest, ) -> Result { - let internal_id = self.internal_session_id(&req.session_id).await?; + let session_id = &req.session_id; let agent = self.get_session_agent(&req.session_id, None).await?; let cancel_token = CancellationToken::new(); let result = agent .extension_manager - .read_resource(&internal_id, &req.uri, &req.extension_name, cancel_token) + .read_resource(session_id, &req.uri, &req.extension_name, cancel_token) .await .internal_err()?; let result_json = serde_json::to_value(&result).internal_err()?; diff --git a/crates/goose/src/acp/server/sessions.rs b/crates/goose/src/acp/server/sessions.rs index f1d21baf6f..517c78f2b1 100644 --- a/crates/goose/src/acp/server/sessions.rs +++ b/crates/goose/src/acp/server/sessions.rs @@ -16,20 +16,15 @@ impl GooseAcpAgent { agent_client_protocol::Error::invalid_params().data("invalid directory path") ); } - let internal_id = self.internal_session_id(&req.session_id).await?; + let session_id = &req.session_id; self.session_manager - .update(&internal_id) + .update(session_id) .working_dir(path.clone()) .apply() .await .internal_err()?; - self.thread_manager - .update_working_dir(&req.session_id, &working_dir) - .await - .internal_err()?; - - if let Some(session) = self.sessions.lock().await.get_mut(&req.session_id) { + if let Some(session) = self.sessions.lock().await.get_mut(session_id) { match &session.agent { AgentHandle::Ready(agent) => { agent.extension_manager.update_working_dir(&path).await; @@ -47,9 +42,8 @@ impl GooseAcpAgent { &self, req: DeleteSessionRequest, ) -> Result { - // Delete the thread and all its internal sessions + messages. - self.thread_manager - .delete_thread(&req.session_id) + self.session_manager + .delete_session(&req.session_id) .await .internal_err()?; self.sessions.lock().await.remove(&req.session_id); @@ -60,17 +54,9 @@ impl GooseAcpAgent { &self, req: ExportSessionRequest, ) -> Result { - let thread = self - .thread_manager - .get_thread(&req.session_id) - .await - .internal_err()?; - let internal_id = thread.current_session_id.ok_or_else(|| { - agent_client_protocol::Error::internal_error().data("Thread has no internal session") - })?; let data = self .session_manager - .export_session(&internal_id) + .export_session(&req.session_id) .await .internal_err()?; Ok(ExportSessionResponse { data }) @@ -82,51 +68,17 @@ impl GooseAcpAgent { ) -> Result { let session = self .session_manager - .import_session(&req.data, Some(SessionType::Acp)) + .import_session(&req.data, None) .await .internal_err()?; - // Create a thread for the imported session. - let thread = self - .thread_manager - .create_thread( - Some(session.name.clone()), - None, - Some(session.working_dir.display().to_string()), - ) - .await - .internal_err()?; - - // Link the internal session to the thread. - self.session_manager - .update(&session.id) - .thread_id(Some(thread.id.clone())) - .apply() - .await - .internal_err()?; - - // Copy conversation messages into thread_messages so they appear in the thread. - if let Some(ref conversation) = session.conversation { - for msg in conversation.messages() { - self.thread_manager - .append_message(&thread.id, Some(&session.id), msg) - .await - .internal_err()?; - } - } - - // Re-fetch thread to get accurate message_count. - let thread = self - .thread_manager - .get_thread(&thread.id) - .await - .internal_err()?; + let msg_count = session.message_count as u64; Ok(ImportSessionResponse { - session_id: thread.id, - title: Some(thread.name), - updated_at: Some(thread.updated_at.to_rfc3339()), - message_count: thread.message_count as u64, + session_id: session.id, + title: Some(session.name), + updated_at: Some(session.updated_at.to_rfc3339()), + message_count: msg_count, }) } @@ -134,11 +86,12 @@ impl GooseAcpAgent { &self, req: UpdateSessionProjectRequest, ) -> Result { - let project_id = req.project_id; - self.update_thread_metadata(&req.session_id, move |meta| { - meta.project_id = project_id; - }) - .await?; + self.session_manager + .update(&req.session_id) + .project_id(req.project_id) + .apply() + .await + .internal_err()?; Ok(EmptyResponse {}) } @@ -146,20 +99,12 @@ impl GooseAcpAgent { &self, req: RenameSessionRequest, ) -> Result { - let title = req.title; - let thread = self - .thread_manager - .update_thread(&req.session_id, Some(title.clone()), Some(true), None) + self.session_manager + .update(&req.session_id) + .user_provided_name(req.title) + .apply() .await .map_err(|e| agent_client_protocol::Error::internal_error().data(e.to_string()))?; - if let Some(internal_session_id) = thread.current_session_id { - self.session_manager - .update(&internal_session_id) - .user_provided_name(title) - .apply() - .await - .map_err(|e| agent_client_protocol::Error::internal_error().data(e.to_string()))?; - } Ok(EmptyResponse {}) } @@ -167,8 +112,10 @@ impl GooseAcpAgent { &self, req: ArchiveSessionRequest, ) -> Result { - self.thread_manager - .archive_thread(&req.session_id) + self.session_manager + .update(&req.session_id) + .archived_at(Some(chrono::Utc::now())) + .apply() .await .internal_err()?; self.sessions.lock().await.remove(&req.session_id); @@ -179,8 +126,10 @@ impl GooseAcpAgent { &self, req: UnarchiveSessionRequest, ) -> Result { - self.thread_manager - .unarchive_thread(&req.session_id) + self.session_manager + .update(&req.session_id) + .archived_at(None) + .apply() .await .internal_err()?; Ok(EmptyResponse {}) diff --git a/crates/goose/src/acp/server/tools.rs b/crates/goose/src/acp/server/tools.rs index a148a5851f..6204f327b5 100644 --- a/crates/goose/src/acp/server/tools.rs +++ b/crates/goose/src/acp/server/tools.rs @@ -7,9 +7,9 @@ impl GooseAcpAgent { &self, req: GetToolsRequest, ) -> Result { - let internal_id = self.internal_session_id(&req.session_id).await?; + let session_id = &req.session_id; let agent = self.get_session_agent(&req.session_id, None).await?; - let tools = agent.list_tools(&internal_id, None).await; + let tools = agent.list_tools(session_id, None).await; let tools_json = tools .into_iter() .map(|t| serde_json::to_value(&t)) @@ -22,9 +22,9 @@ impl GooseAcpAgent { &self, req: GooseToolCallRequest, ) -> Result { - let internal_id = self.internal_session_id(&req.session_id).await?; + let session_id = &req.session_id; let agent = self.get_session_agent(&req.session_id, None).await?; - let tools = agent.list_tools(&internal_id, None).await; + let tools = agent.list_tools(session_id, None).await; let Some(tool) = tools.iter().find(|t| *t.name == req.name) else { return Err(agent_client_protocol::Error::invalid_params().data("tool not found")); @@ -52,7 +52,7 @@ impl GooseAcpAgent { params }; - let ctx = crate::agents::ToolCallContext::new(internal_id, None, None); + let ctx = crate::agents::ToolCallContext::new(session_id.clone(), None, None); let tool_result = agent .extension_manager .dispatch_tool_call(&ctx, tool_call, CancellationToken::new()) diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index 5a8d97da2e..f2ffddff14 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -367,11 +367,7 @@ impl Agent { } async fn load_project_instructions(&self, session: &Session) -> Option { - let thread_id = session.thread_id.as_deref()?; - let thread_mgr = - crate::session::ThreadManager::new(self.config.session_manager.storage().clone()); - let thread = thread_mgr.get_thread(thread_id).await.ok()?; - let project_id = thread.metadata.project_id.as_deref()?; + let project_id = session.project_id.as_deref()?; let entry = crate::sources::read_project(project_id).ok()?; let mut parts = Vec::new(); parts.push(format!("# Project: {}", entry.name)); diff --git a/crates/goose/src/execution/manager.rs b/crates/goose/src/execution/manager.rs index a8e199b151..d590ca53a5 100644 --- a/crates/goose/src/execution/manager.rs +++ b/crates/goose/src/execution/manager.rs @@ -382,28 +382,6 @@ mod tests { assert_eq!(manager.session_count().await, 1); } - #[tokio::test] - async fn test_set_default_provider() { - use crate::providers::testprovider::TestProvider; - - let temp_dir = TempDir::new().unwrap(); - let manager = create_test_manager(&temp_dir).await; - - // Create a test provider for replaying (doesn't need inner provider) - let temp_file = temp_dir.path().join("test_provider.json"); - - // Create an empty test provider (will fail on actual use but that's ok for this test) - std::fs::write(&temp_file, "{}").unwrap(); - let test_provider = TestProvider::new_replaying(temp_file.to_str().unwrap()).unwrap(); - - manager.set_default_provider(Arc::new(test_provider)).await; - - let session = String::from("provider-test"); - let _agent = manager.get_or_create_agent(session.clone()).await.unwrap(); - - assert!(manager.has_session(&session).await); - } - #[tokio::test] async fn test_eviction_updates_last_used() { // Test that accessing a session updates its last_used timestamp diff --git a/crates/goose/src/session/mod.rs b/crates/goose/src/session/mod.rs index e7ba138e8c..64dfdedcc4 100644 --- a/crates/goose/src/session/mod.rs +++ b/crates/goose/src/session/mod.rs @@ -3,7 +3,6 @@ mod diagnostics; pub mod extension_data; mod legacy; pub mod session_manager; -pub mod thread_manager; pub use diagnostics::{ config_path, generate_diagnostics, get_system_info, latest_llm_log_path, @@ -13,4 +12,3 @@ pub use extension_data::{EnabledExtensionsState, ExtensionData, ExtensionState, pub use session_manager::{ Session, SessionInsights, SessionManager, SessionNameUpdate, SessionType, SessionUpdateBuilder, }; -pub use thread_manager::{Thread, ThreadManager, ThreadMetadata}; diff --git a/crates/goose/src/session/session_manager.rs b/crates/goose/src/session/session_manager.rs index 966685a3da..de0efe66ec 100644 --- a/crates/goose/src/session/session_manager.rs +++ b/crates/goose/src/session/session_manager.rs @@ -19,7 +19,7 @@ use std::sync::{Arc, LazyLock}; use tracing::{info, warn}; use utoipa::ToSchema; -pub const CURRENT_SCHEMA_VERSION: i32 = 11; +pub const CURRENT_SCHEMA_VERSION: i32 = 12; pub const SESSIONS_FOLDER: &str = "sessions"; pub const DB_NAME: &str = "sessions.db"; @@ -82,7 +82,9 @@ pub struct Session { #[serde(default)] pub goose_mode: GooseMode, #[serde(default)] - pub thread_id: Option, + pub archived_at: Option>, + #[serde(default)] + pub project_id: Option, } pub struct SessionUpdateBuilder<'a> { @@ -105,7 +107,9 @@ pub struct SessionUpdateBuilder<'a> { provider_name: Option>, model_config: Option>, goose_mode: Option, - thread_id: Option>, + archived_at: Option>>, + + project_id: Option>, } #[derive(Serialize, ToSchema, Debug)] @@ -137,7 +141,8 @@ impl<'a> SessionUpdateBuilder<'a> { provider_name: None, model_config: None, goose_mode: None, - thread_id: None, + archived_at: None, + project_id: None, } } @@ -246,8 +251,13 @@ impl<'a> SessionUpdateBuilder<'a> { self } - pub fn thread_id(mut self, thread_id: Option) -> Self { - self.thread_id = Some(thread_id); + pub fn archived_at(mut self, archived_at: Option>) -> Self { + self.archived_at = Some(archived_at); + self + } + + pub fn project_id(mut self, project_id: Option) -> Self { + self.project_id = Some(project_id); self } } @@ -258,7 +268,11 @@ pub struct SessionManager { #[derive(Debug, Clone)] pub struct SessionNameUpdate { - pub thread: super::thread_manager::Thread, + pub session_id: String, + pub name: String, + pub updated_at: chrono::DateTime, + pub message_count: usize, + pub user_set_name: bool, } impl SessionManager { @@ -384,21 +398,16 @@ impl SessionManager { .apply() .await?; - // Also update the thread name so ACP clients see it via session/list. - if let Some(ref thread_id) = session.thread_id { - let thread_mgr = super::thread_manager::ThreadManager::new(self.storage.clone()); - let thread = thread_mgr.get_thread(thread_id).await?; - if !thread.user_set_name && thread.name != name { - let thread = thread_mgr - .update_thread(thread_id, Some(name), Some(false), None) - .await?; - return Ok(Some(SessionNameUpdate { thread })); - } - } - Ok(None) - } else { - Ok(None) + let session = self.get_session(id, false).await?; + return Ok(Some(SessionNameUpdate { + session_id: id.to_string(), + name, + updated_at: session.updated_at, + message_count: session.message_count, + user_set_name: session.user_set_name, + })); } + Ok(None) } pub async fn search_chat_history( @@ -433,6 +442,22 @@ impl SessionManager { .update_message_metadata(id, message_id, f) .await } + + /// Patch `tool_meta` on a specific `ToolRequest` within a stored message. + /// Used to persist LLM-generated tool titles and chain summaries so they + /// survive session reload. Merge-based: existing keys not in `patch` are + /// preserved. No-op if the message or tool_call_id is not found. + pub async fn update_tool_request_meta( + &self, + session_id: &str, + message_id: &str, + tool_call_id: &str, + patch: serde_json::Value, + ) -> Result<()> { + self.storage + .update_tool_request_meta(session_id, message_id, tool_call_id, patch) + .await + } } pub struct SessionStorage { @@ -473,7 +498,8 @@ impl Default for Session { provider_name: None, model_config: None, goose_mode: GooseMode::default(), - thread_id: None, + archived_at: None, + project_id: None, } } } @@ -543,7 +569,8 @@ impl sqlx::FromRow<'_, sqlx::sqlite::SqliteRow> for Session { .ok() .and_then(|s| s.parse().ok()) .unwrap_or_default(), - thread_id: row.try_get("thread_id").ok().flatten(), + archived_at: row.try_get("archived_at").ok(), + project_id: row.try_get("project_id").ok().flatten(), }) } } @@ -644,7 +671,8 @@ impl SessionStorage { provider_name TEXT, model_config_json TEXT, goose_mode TEXT NOT NULL DEFAULT 'auto', - thread_id TEXT + archived_at TIMESTAMP, + project_id TEXT ) "#, ) @@ -684,49 +712,6 @@ impl SessionStorage { sqlx::query("CREATE INDEX idx_sessions_type ON sessions(session_type)") .execute(pool) .await?; - sqlx::query("CREATE INDEX IF NOT EXISTS idx_sessions_thread ON sessions(thread_id)") - .execute(pool) - .await?; - - sqlx::query( - "CREATE TABLE IF NOT EXISTS threads ( - id TEXT PRIMARY KEY, - name TEXT NOT NULL DEFAULT 'New Chat', - user_set_name BOOLEAN DEFAULT FALSE, - working_dir TEXT, - created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, - updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, - archived_at TIMESTAMP, - metadata_json TEXT DEFAULT '{}' - )", - ) - .execute(pool) - .await?; - - sqlx::query( - "CREATE TABLE IF NOT EXISTS thread_messages ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - thread_id TEXT NOT NULL REFERENCES threads(id), - session_id TEXT, - message_id TEXT, - role TEXT NOT NULL, - content_json TEXT NOT NULL, - created_timestamp INTEGER NOT NULL, - metadata_json TEXT DEFAULT '{}' - )", - ) - .execute(pool) - .await?; - - sqlx::query( - "CREATE INDEX IF NOT EXISTS idx_thread_messages_thread ON thread_messages(thread_id)", - ) - .execute(pool) - .await?; - sqlx::query("CREATE INDEX IF NOT EXISTS idx_thread_messages_message_id ON thread_messages(message_id)") - .execute(pool) - .await?; - crate::providers::inventory::create_tables(pool).await?; Ok(()) @@ -1075,6 +1060,46 @@ impl SessionStorage { 11 => { crate::providers::inventory::create_tables_in_tx(tx).await?; } + 12 => { + // Add archived_at, project_id columns to sessions. + let has_archived_at = sqlx::query_scalar::<_, i32>( + "SELECT COUNT(*) FROM pragma_table_info('sessions') WHERE name = 'archived_at'", + ) + .fetch_one(&mut **tx) + .await? + > 0; + if !has_archived_at { + sqlx::query("ALTER TABLE sessions ADD COLUMN archived_at TIMESTAMP") + .execute(&mut **tx) + .await?; + } + + let has_project_id = sqlx::query_scalar::<_, i32>( + "SELECT COUNT(*) FROM pragma_table_info('sessions') WHERE name = 'project_id'", + ) + .fetch_one(&mut **tx) + .await? + > 0; + if !has_project_id { + sqlx::query("ALTER TABLE sessions ADD COLUMN project_id TEXT") + .execute(&mut **tx) + .await?; + } + + // Drop thread tables and thread_id column (no longer used). + sqlx::query("DROP TABLE IF EXISTS thread_messages") + .execute(&mut **tx) + .await?; + sqlx::query("DROP TABLE IF EXISTS threads") + .execute(&mut **tx) + .await?; + sqlx::query("DROP INDEX IF EXISTS idx_sessions_thread") + .execute(&mut **tx) + .await?; + sqlx::query("ALTER TABLE sessions DROP COLUMN thread_id") + .execute(&mut **tx) + .await?; + } _ => { anyhow::bail!("Unknown migration version: {}", version); } @@ -1136,7 +1161,8 @@ impl SessionStorage { total_tokens, input_tokens, output_tokens, accumulated_total_tokens, accumulated_input_tokens, accumulated_output_tokens, schedule_id, recipe_json, user_recipe_values_json, - provider_name, model_config_json, goose_mode, thread_id + provider_name, model_config_json, goose_mode, + archived_at, project_id FROM sessions WHERE id = ? "#, @@ -1200,7 +1226,9 @@ impl SessionStorage { add_update!(builder.provider_name, "provider_name"); add_update!(builder.model_config, "model_config_json"); add_update!(builder.goose_mode, "goose_mode"); - add_update!(builder.thread_id, "thread_id"); + add_update!(builder.archived_at, "archived_at"); + + add_update!(builder.project_id, "project_id"); if updates.is_empty() { return Ok(()); @@ -1269,14 +1297,22 @@ impl SessionStorage { if let Some(goose_mode) = builder.goose_mode { q = q.bind(goose_mode.to_string()); } - if let Some(thread_id) = builder.thread_id { - q = q.bind(thread_id); + if let Some(ref archived_at) = builder.archived_at { + q = q.bind(archived_at.as_ref()); + } + + if let Some(ref project_id) = builder.project_id { + q = q.bind(project_id.as_ref()); } let pool = self.pool().await?; let mut tx = pool.begin_with("BEGIN IMMEDIATE").await?; q = q.bind(&builder.session_id); - q.execute(&mut *tx).await?; + let result = q.execute(&mut *tx).await?; + + if result.rows_affected() == 0 { + return Err(anyhow::anyhow!("Session not found: {}", builder.session_id)); + } tx.commit().await?; Ok(()) @@ -1423,7 +1459,8 @@ impl SessionStorage { s.total_tokens, s.input_tokens, s.output_tokens, s.accumulated_total_tokens, s.accumulated_input_tokens, s.accumulated_output_tokens, s.schedule_id, s.recipe_json, s.user_recipe_values_json, - s.provider_name, s.model_config_json, s.goose_mode, s.thread_id, + s.provider_name, s.model_config_json, s.goose_mode, + s.archived_at, s.project_id, COUNT(m.id) as message_count FROM sessions s LEFT JOIN messages m ON s.id = m.session_id @@ -1582,15 +1619,15 @@ impl SessionStorage { .recipe(original_session.recipe) .user_recipe_values(original_session.user_recipe_values); - // Preserve provider, model config, and goose_mode from original session + if let Some(project_id) = original_session.project_id { + builder = builder.project_id(Some(project_id)); + } if let Some(provider_name) = original_session.provider_name { builder = builder.provider_name(provider_name); } - if let Some(model_config) = original_session.model_config { builder = builder.model_config(model_config); } - builder = builder.goose_mode(original_session.goose_mode); builder.apply().await?; @@ -1680,6 +1717,81 @@ impl SessionStorage { Ok(()) } + + /// Patch `tool_meta` on a specific `ToolRequest` within a stored message's + /// `content_json`. Finds the row(s) with matching `message_id`, scans each + /// row's content for a `ToolRequest` with the given `tool_call_id`, and + /// merges `patch` into its `tool_meta`. Uses `BEGIN IMMEDIATE` so + /// concurrent writers serialize correctly. + async fn update_tool_request_meta( + &self, + session_id: &str, + message_id: &str, + tool_call_id: &str, + patch: serde_json::Value, + ) -> Result<()> { + use crate::conversation::message::MessageContent; + + let pool = self.pool().await?; + let mut tx = pool.begin_with("BEGIN IMMEDIATE").await?; + + let rows = sqlx::query_as::<_, (i64, String)>( + "SELECT id, content_json FROM messages \ + WHERE session_id = ? AND message_id = ? \ + ORDER BY id ASC", + ) + .bind(session_id) + .bind(message_id) + .fetch_all(&mut *tx) + .await?; + + for (row_id, content_json) in rows { + let mut content: Vec = serde_json::from_str(&content_json)?; + let mut found = false; + for block in &mut content { + if let MessageContent::ToolRequest(tr) = block { + if tr.id == tool_call_id { + tr.tool_meta = Some(merge_tool_meta(tr.tool_meta.take(), &patch)); + found = true; + break; + } + } + } + if !found { + continue; + } + + let updated_json = serde_json::to_string(&content)?; + sqlx::query("UPDATE messages SET content_json = ? WHERE id = ?") + .bind(updated_json) + .bind(row_id) + .execute(&mut *tx) + .await?; + tx.commit().await?; + return Ok(()); + } + + tx.commit().await?; + Ok(()) + } +} + +/// Merge a JSON object `patch` into an existing optional object value, +/// preserving keys not present in the patch. +fn merge_tool_meta( + existing: Option, + patch: &serde_json::Value, +) -> serde_json::Value { + let mut base = match existing { + Some(serde_json::Value::Object(map)) => map, + _ => serde_json::Map::new(), + }; + if let serde_json::Value::Object(patch_map) = patch { + for (k, v) in patch_map { + base.insert(k.clone(), v.clone()); + } + } + serde_json::Value::Object(base) } #[cfg(test)] diff --git a/crates/goose/src/session/thread_manager.rs b/crates/goose/src/session/thread_manager.rs deleted file mode 100644 index 2010603771..0000000000 --- a/crates/goose/src/session/thread_manager.rs +++ /dev/null @@ -1,947 +0,0 @@ -use super::session_manager::{role_to_string, SessionStorage}; -use crate::conversation::message::{Message, MessageContent}; -use anyhow::Result; -use chrono::{DateTime, Utc}; -use rmcp::model::Role; -use serde::{Deserialize, Serialize}; -use std::sync::Arc; - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct Thread { - pub id: String, - pub name: String, - pub user_set_name: bool, - pub working_dir: Option, - pub created_at: DateTime, - pub updated_at: DateTime, - pub archived_at: Option>, - pub metadata: ThreadMetadata, - #[serde(default)] - pub current_session_id: Option, - #[serde(default)] - pub message_count: i64, -} - -#[derive(Debug, Clone, Default, Serialize, Deserialize)] -pub struct ThreadMetadata { - #[serde(default)] - pub persona_id: Option, - #[serde(default)] - pub project_id: Option, - #[serde(default)] - pub provider_id: Option, - #[serde(default, alias = "model_name")] - pub model_id: Option, - #[serde(default)] - pub mode: Option, - #[serde(flatten)] - pub extra: std::collections::HashMap, -} - -pub struct ThreadManager { - storage: Arc, -} - -const THREAD_SELECT: &str = "\ - SELECT t.id, t.name, t.user_set_name, t.working_dir, t.created_at, t.updated_at, \ - t.archived_at, t.metadata_json, \ - (SELECT s.id FROM sessions s WHERE s.thread_id = t.id ORDER BY s.created_at DESC LIMIT 1) as current_session_id, \ - (SELECT COUNT(*) FROM thread_messages WHERE thread_id = t.id) as message_count \ - FROM threads t"; - -type ThreadRow = ( - String, - String, - bool, - Option, - DateTime, - DateTime, - Option>, - String, - Option, - i64, -); - -fn thread_from_row( - ( - id, - name, - user_set_name, - working_dir, - created_at, - updated_at, - archived_at, - metadata_json, - current_session_id, - message_count, - ): ThreadRow, -) -> Result { - let metadata: ThreadMetadata = serde_json::from_str(&metadata_json).unwrap_or_default(); - - Ok(Thread { - id, - name, - user_set_name, - working_dir, - created_at, - updated_at, - archived_at, - metadata, - current_session_id, - message_count, - }) -} - -impl ThreadManager { - pub fn new(storage: Arc) -> Self { - Self { storage } - } - - pub async fn create_thread( - &self, - name: Option, - metadata: Option, - working_dir: Option, - ) -> Result { - let pool = self.storage.pool().await?; - let id = uuid::Uuid::new_v4().to_string(); - let name = name.unwrap_or_else(|| "New Chat".to_string()); - let meta = metadata.unwrap_or_default(); - let metadata_json = serde_json::to_string(&meta)?; - - sqlx::query( - "INSERT INTO threads (id, name, user_set_name, working_dir, metadata_json) VALUES (?, ?, FALSE, ?, ?)", - ) - .bind(&id) - .bind(&name) - .bind(&working_dir) - .bind(&metadata_json) - .execute(pool) - .await?; - - self.get_thread(&id).await - } - - pub async fn get_thread(&self, id: &str) -> Result { - let pool = self.storage.pool().await?; - let sql = format!("{} WHERE t.id = ?", THREAD_SELECT); - let row = sqlx::query_as::<_, ThreadRow>(&sql) - .bind(id) - .fetch_one(pool) - .await?; - - thread_from_row(row) - } - - pub async fn update_thread( - &self, - id: &str, - name: Option, - user_set_name: Option, - metadata: Option, - ) -> Result { - let pool = self.storage.pool().await?; - let mut sets = Vec::new(); - - if name.is_some() { - sets.push("name = ?"); - sets.push("user_set_name = ?"); - } - if metadata.is_some() { - sets.push("metadata_json = ?"); - } - - if !sets.is_empty() { - let sql = format!( - "UPDATE threads SET {}, updated_at = CURRENT_TIMESTAMP WHERE id = ?", - sets.join(", ") - ); - let mut q = sqlx::query(&sql); - if let Some(ref n) = name { - q = q.bind(n); - q = q.bind(user_set_name.unwrap_or(true)); - } - if let Some(ref meta) = metadata { - q = q.bind(serde_json::to_string(meta)?); - } - q = q.bind(id); - q.execute(pool).await?; - } - - self.get_thread(id).await - } - - pub async fn list_threads(&self, include_archived: bool) -> Result> { - let pool = self.storage.pool().await?; - let sql = if include_archived { - format!("{} ORDER BY t.updated_at DESC", THREAD_SELECT) - } else { - format!( - "{} WHERE t.archived_at IS NULL ORDER BY t.updated_at DESC", - THREAD_SELECT - ) - }; - let rows = sqlx::query_as::<_, ThreadRow>(&sql).fetch_all(pool).await?; - - rows.into_iter().map(thread_from_row).collect() - } - - pub async fn archive_thread(&self, id: &str) -> Result { - let pool = self.storage.pool().await?; - sqlx::query("UPDATE threads SET archived_at = CURRENT_TIMESTAMP, updated_at = CURRENT_TIMESTAMP WHERE id = ?") - .bind(id) - .execute(pool) - .await?; - self.get_thread(id).await - } - - pub async fn unarchive_thread(&self, id: &str) -> Result { - let pool = self.storage.pool().await?; - sqlx::query( - "UPDATE threads SET archived_at = NULL, updated_at = CURRENT_TIMESTAMP WHERE id = ?", - ) - .bind(id) - .execute(pool) - .await?; - self.get_thread(id).await - } - - pub async fn update_metadata( - &self, - id: &str, - f: impl FnOnce(&mut ThreadMetadata), - ) -> Result { - let thread = self.get_thread(id).await?; - let mut meta = thread.metadata; - f(&mut meta); - self.update_thread(id, None, None, Some(meta)).await - } - - pub async fn update_working_dir(&self, id: &str, working_dir: &str) -> Result<()> { - let pool = self.storage.pool().await?; - sqlx::query( - "UPDATE threads SET working_dir = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?", - ) - .bind(working_dir) - .bind(id) - .execute(pool) - .await?; - Ok(()) - } - - pub async fn delete_thread(&self, id: &str) -> Result<()> { - let pool = self.storage.pool().await?; - let mut tx = pool.begin().await?; - - sqlx::query("DELETE FROM thread_messages WHERE thread_id = ?") - .bind(id) - .execute(&mut *tx) - .await?; - sqlx::query( - "DELETE FROM messages WHERE session_id IN (SELECT id FROM sessions WHERE thread_id = ?)", - ) - .bind(id) - .execute(&mut *tx) - .await?; - sqlx::query("DELETE FROM sessions WHERE thread_id = ?") - .bind(id) - .execute(&mut *tx) - .await?; - sqlx::query("DELETE FROM threads WHERE id = ?") - .bind(id) - .execute(&mut *tx) - .await?; - - tx.commit().await?; - Ok(()) - } - - pub async fn append_message( - &self, - thread_id: &str, - session_id: Option<&str>, - message: &Message, - ) -> Result { - let pool = self.storage.pool().await?; - let role_str = role_to_string(&message.role); - let metadata_json = serde_json::to_string(&message.metadata)?; - - // When the incoming message is text-only, try to coalesce it with the - // last stored row if that row has the same role and is also text-only. - // This avoids storing one row per streaming token while keeping the UI - // streaming path unchanged (callers still forward every chunk). - if message.has_only_text_content() && !message.content.is_empty() { - let new_text = message.as_concat_text(); - - let maybe_last = sqlx::query_as::<_, (i64, String, String, String, String)>( - "SELECT id, message_id, role, content_json, metadata_json \ - FROM thread_messages \ - WHERE thread_id = ? \ - ORDER BY id DESC LIMIT 1", - ) - .bind(thread_id) - .fetch_optional(pool) - .await?; - - if let Some(( - row_id, - existing_msg_id, - last_role, - last_content_json, - last_metadata_json, - )) = maybe_last - { - if last_role == role_str - && last_metadata_json == metadata_json - && is_text_only_json(&last_content_json) - { - // Append text into the existing row's single text element. - let updated_json = append_text_json(&last_content_json, &new_text)?; - sqlx::query("UPDATE thread_messages SET content_json = ? WHERE id = ?") - .bind(&updated_json) - .bind(row_id) - .execute(pool) - .await?; - - sqlx::query("UPDATE threads SET updated_at = CURRENT_TIMESTAMP WHERE id = ?") - .bind(thread_id) - .execute(pool) - .await?; - - let mut stored = message.clone(); - stored.id = Some(existing_msg_id); - return Ok(stored); - } - } - } - - // Default path: insert a new row. - let content_json = serde_json::to_string(&message.content)?; - - let message_id = message - .id - .clone() - .unwrap_or_else(|| format!("tmsg_{}", uuid::Uuid::new_v4())); - - sqlx::query( - "INSERT INTO thread_messages (thread_id, session_id, message_id, role, content_json, created_timestamp, metadata_json) VALUES (?, ?, ?, ?, ?, ?, ?)", - ) - .bind(thread_id) - .bind(session_id) - .bind(&message_id) - .bind(role_str) - .bind(&content_json) - .bind(message.created) - .bind(&metadata_json) - .execute(pool) - .await?; - - sqlx::query("UPDATE threads SET updated_at = CURRENT_TIMESTAMP WHERE id = ?") - .bind(thread_id) - .execute(pool) - .await?; - - let mut stored = message.clone(); - stored.id = Some(message_id); - Ok(stored) - } - - pub async fn fork_thread(&self, source_thread_id: &str) -> Result { - let source = self.get_thread(source_thread_id).await?; - let pool = self.storage.pool().await?; - - let new_id = uuid::Uuid::new_v4().to_string(); - let name = format!("Fork of {}", source.name); - let metadata_json = serde_json::to_string(&source.metadata)?; - - sqlx::query( - "INSERT INTO threads (id, name, user_set_name, working_dir, metadata_json) VALUES (?, ?, FALSE, ?, ?)", - ) - .bind(&new_id) - .bind(&name) - .bind(&source.working_dir) - .bind(&metadata_json) - .execute(pool) - .await?; - - // Copy all thread messages - sqlx::query( - "INSERT INTO thread_messages (thread_id, session_id, message_id, role, content_json, created_timestamp, metadata_json) \ - SELECT ?, session_id, 'tmsg_' || hex(randomblob(16)), role, content_json, created_timestamp, metadata_json \ - FROM thread_messages WHERE thread_id = ? ORDER BY id ASC", - ) - .bind(&new_id) - .bind(source_thread_id) - .execute(pool) - .await?; - - self.get_thread(&new_id).await - } - - /// Merge a JSON object patch into the `tool_meta` of the `ToolRequest` whose - /// `id == tool_call_id` inside the message identified by `(thread_id, - /// message_id)`. Existing keys in `tool_meta` are preserved. - /// - /// No-ops (returns `Ok(())`) if the row containing the tool request can't - /// be found — callers (e.g. async title tasks) treat persistence as - /// best-effort. - /// - /// `message_id` is used as a coarse filter, but multiple `thread_messages` - /// rows can share the same `message_id` when the agent splits a single - /// LLM response (e.g. text + tool_request) into separate - /// `AgentEvent::Message` events. We disambiguate by walking the matching - /// rows and picking the one whose content actually contains a - /// `ToolRequest` with `tool_call_id`, then update only that row by its - /// auto-incremented primary key. Without this, the title for the first - /// tool in such a split message never persists, because `fetch_optional` - /// returns the text-only row first and finds no matching tool call. - pub async fn update_tool_request_meta( - &self, - thread_id: &str, - message_id: &str, - tool_call_id: &str, - patch: serde_json::Value, - ) -> Result<()> { - let pool = self.storage.pool().await?; - let mut tx = pool.begin_with("BEGIN IMMEDIATE").await?; - - let rows = sqlx::query_as::<_, (i64, String)>( - "SELECT id, content_json FROM thread_messages \ - WHERE thread_id = ? AND message_id = ? \ - ORDER BY id ASC", - ) - .bind(thread_id) - .bind(message_id) - .fetch_all(&mut *tx) - .await?; - - for (row_id, content_json) in rows { - let mut content: Vec = serde_json::from_str(&content_json)?; - let mut found = false; - for block in &mut content { - if let MessageContent::ToolRequest(tr) = block { - if tr.id == tool_call_id { - tr.tool_meta = Some(merge_tool_meta(tr.tool_meta.take(), &patch)); - found = true; - break; - } - } - } - if !found { - continue; - } - - let updated_json = serde_json::to_string(&content)?; - sqlx::query("UPDATE thread_messages SET content_json = ? WHERE id = ?") - .bind(updated_json) - .bind(row_id) - .execute(&mut *tx) - .await?; - tx.commit().await?; - return Ok(()); - } - - tx.commit().await?; - Ok(()) - } - - pub async fn list_messages(&self, thread_id: &str) -> Result> { - let pool = self.storage.pool().await?; - let rows = sqlx::query_as::<_, (Option, String, Option, String, i64, String)>( - "SELECT message_id, role, session_id, content_json, created_timestamp, metadata_json FROM thread_messages WHERE thread_id = ? ORDER BY id ASC", - ) - .bind(thread_id) - .fetch_all(pool) - .await?; - - let mut messages = Vec::new(); - for (message_id, role_str, _session_id, content_json, created_timestamp, metadata_json) in - rows - { - let role = match role_str.as_str() { - "user" => Role::User, - "assistant" => Role::Assistant, - _ => continue, - }; - let content = serde_json::from_str(&content_json)?; - let metadata = serde_json::from_str(&metadata_json).unwrap_or_default(); - - let mut msg = Message::new(role, created_timestamp, content); - msg.metadata = metadata; - if let Some(id) = message_id { - msg = msg.with_id(id); - } - messages.push(msg); - } - - Ok(messages) - } -} - -/// Check whether a `content_json` string represents a single text-only element. -/// Avoids a full deserialize by inspecting the JSON structure directly. -fn is_text_only_json(content_json: &str) -> bool { - let Ok(items) = serde_json::from_str::>(content_json) else { - return false; - }; - items.len() == 1 - && items[0].get("type").and_then(|v| v.as_str()) == Some("text") - && items[0].get("text").is_some() -} - -/// Append `new_text` to the single text element in a text-only `content_json` array. -fn append_text_json(content_json: &str, new_text: &str) -> anyhow::Result { - let mut items: Vec = serde_json::from_str(content_json)?; - if let Some(text_val) = items.get_mut(0).and_then(|v| v.get_mut("text")) { - let existing = text_val.as_str().unwrap_or(""); - *text_val = serde_json::Value::String(format!("{}{}", existing, new_text)); - } - Ok(serde_json::to_string(&items)?) -} - -/// Merge a JSON object `patch` into an existing optional object value, -/// preserving keys not present in the patch. Non-object values are replaced. -fn merge_tool_meta( - existing: Option, - patch: &serde_json::Value, -) -> serde_json::Value { - let mut base = match existing { - Some(serde_json::Value::Object(map)) => map, - _ => serde_json::Map::new(), - }; - if let serde_json::Value::Object(patch_map) = patch { - for (k, v) in patch_map { - base.insert(k.clone(), v.clone()); - } - } - serde_json::Value::Object(base) -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::conversation::message::{ - Message, MessageContent, ToolRequest, TOOL_META_CHAIN_SUMMARY_KEY, - TOOL_META_EXTERNAL_DISPATCH_KEY, TOOL_META_TITLE_KEY, - }; - use crate::session::SessionManager; - use rmcp::model::CallToolRequestParams; - use tempfile::TempDir; - - fn assistant_message_with_tool_request( - tool_id: &str, - tool_meta: Option, - ) -> Message { - let tool_request = ToolRequest { - id: tool_id.to_string(), - tool_call: Ok(CallToolRequestParams::new("developer__shell")), - metadata: None, - tool_meta, - }; - Message::new( - Role::Assistant, - chrono::Utc::now().timestamp_millis(), - vec![MessageContent::ToolRequest(tool_request)], - ) - } - - async fn fresh_thread_manager(temp: &TempDir) -> Arc { - let session_manager = SessionManager::new(temp.path().to_path_buf()); - Arc::new(ThreadManager::new(session_manager.storage().clone())) - } - - #[tokio::test] - async fn update_tool_request_meta_sets_title_when_missing() { - let temp = TempDir::new().unwrap(); - let mgr = fresh_thread_manager(&temp).await; - let thread = mgr.create_thread(None, None, None).await.unwrap(); - - let stored = mgr - .append_message( - &thread.id, - None, - &assistant_message_with_tool_request("tc-1", None), - ) - .await - .unwrap(); - let message_id = stored.id.clone().unwrap(); - - mgr.update_tool_request_meta( - &thread.id, - &message_id, - "tc-1", - serde_json::json!({ TOOL_META_TITLE_KEY: "reading config" }), - ) - .await - .unwrap(); - - let messages = mgr.list_messages(&thread.id).await.unwrap(); - let req = match &messages[0].content[0] { - MessageContent::ToolRequest(r) => r, - _ => panic!("expected tool request"), - }; - assert_eq!(req.persisted_title(), Some("reading config")); - } - - #[tokio::test] - async fn update_tool_request_meta_preserves_existing_keys() { - let temp = TempDir::new().unwrap(); - let mgr = fresh_thread_manager(&temp).await; - let thread = mgr.create_thread(None, None, None).await.unwrap(); - - let stored = mgr - .append_message( - &thread.id, - None, - &assistant_message_with_tool_request( - "tc-1", - Some(serde_json::json!({ TOOL_META_EXTERNAL_DISPATCH_KEY: true })), - ), - ) - .await - .unwrap(); - let message_id = stored.id.clone().unwrap(); - - mgr.update_tool_request_meta( - &thread.id, - &message_id, - "tc-1", - serde_json::json!({ TOOL_META_TITLE_KEY: "running commands" }), - ) - .await - .unwrap(); - - let messages = mgr.list_messages(&thread.id).await.unwrap(); - let req = match &messages[0].content[0] { - MessageContent::ToolRequest(r) => r, - _ => panic!("expected tool request"), - }; - assert!( - req.is_externally_dispatched(), - "external_dispatch key should be preserved across the merge" - ); - assert_eq!(req.persisted_title(), Some("running commands")); - } - - #[tokio::test] - async fn update_tool_request_meta_overwrites_existing_value() { - let temp = TempDir::new().unwrap(); - let mgr = fresh_thread_manager(&temp).await; - let thread = mgr.create_thread(None, None, None).await.unwrap(); - - let stored = mgr - .append_message( - &thread.id, - None, - &assistant_message_with_tool_request( - "tc-1", - Some(serde_json::json!({ TOOL_META_TITLE_KEY: "old" })), - ), - ) - .await - .unwrap(); - let message_id = stored.id.clone().unwrap(); - - mgr.update_tool_request_meta( - &thread.id, - &message_id, - "tc-1", - serde_json::json!({ TOOL_META_TITLE_KEY: "new" }), - ) - .await - .unwrap(); - - let messages = mgr.list_messages(&thread.id).await.unwrap(); - let req = match &messages[0].content[0] { - MessageContent::ToolRequest(r) => r, - _ => panic!("expected tool request"), - }; - assert_eq!(req.persisted_title(), Some("new")); - } - - #[tokio::test] - async fn update_tool_request_meta_no_op_for_unknown_message() { - let temp = TempDir::new().unwrap(); - let mgr = fresh_thread_manager(&temp).await; - let thread = mgr.create_thread(None, None, None).await.unwrap(); - - mgr.update_tool_request_meta( - &thread.id, - "missing-message-id", - "tc-1", - serde_json::json!({ TOOL_META_TITLE_KEY: "x" }), - ) - .await - .expect("missing message must be a no-op, not an error"); - } - - #[tokio::test] - async fn update_tool_request_meta_no_op_for_unknown_tool_call() { - let temp = TempDir::new().unwrap(); - let mgr = fresh_thread_manager(&temp).await; - let thread = mgr.create_thread(None, None, None).await.unwrap(); - - let stored = mgr - .append_message( - &thread.id, - None, - &assistant_message_with_tool_request("tc-1", None), - ) - .await - .unwrap(); - let message_id = stored.id.clone().unwrap(); - - mgr.update_tool_request_meta( - &thread.id, - &message_id, - "tc-other", - serde_json::json!({ TOOL_META_TITLE_KEY: "x" }), - ) - .await - .unwrap(); - - let messages = mgr.list_messages(&thread.id).await.unwrap(); - let req = match &messages[0].content[0] { - MessageContent::ToolRequest(r) => r, - _ => panic!("expected tool request"), - }; - assert!( - req.persisted_title().is_none(), - "no-match must leave tool_meta untouched" - ); - } - - #[tokio::test] - async fn update_tool_request_meta_targets_correct_row_when_message_id_is_shared() { - // Regression for "first tool call in a chain consistently shows the - // deterministic title on reload." Bedrock/Anthropic-style streaming - // produces a single LLM message id (e.g. `msg_bdrk_…`) but the agent - // splits it across multiple `AgentEvent::Message` events — one for - // text, one for the trailing tool_request — and `append_message` - // writes a separate row per event. Both rows end up with the SAME - // `message_id`. `fetch_optional` returned the text-only row first and - // the title never persisted. - use crate::conversation::message::ToolRequest; - use rmcp::model::CallToolRequestParams; - - let temp = TempDir::new().unwrap(); - let mgr = fresh_thread_manager(&temp).await; - let thread = mgr.create_thread(None, None, None).await.unwrap(); - - let shared_id = "msg_bdrk_shared".to_string(); - - let mut text_only = Message::new( - Role::Assistant, - chrono::Utc::now().timestamp_millis(), - vec![MessageContent::text( - "Let me look at the project structure.", - )], - ); - text_only.id = Some(shared_id.clone()); - let stored_text = mgr - .append_message(&thread.id, None, &text_only) - .await - .unwrap(); - assert_eq!(stored_text.id.as_deref(), Some(shared_id.as_str())); - - let mut tool_message = Message::new( - Role::Assistant, - chrono::Utc::now().timestamp_millis(), - vec![MessageContent::ToolRequest(ToolRequest { - id: "toolu_tree".to_string(), - tool_call: Ok(CallToolRequestParams::new("tree")), - metadata: None, - tool_meta: None, - })], - ); - tool_message.id = Some(shared_id.clone()); - let stored_tool = mgr - .append_message(&thread.id, None, &tool_message) - .await - .unwrap(); - assert_eq!(stored_tool.id.as_deref(), Some(shared_id.as_str())); - - mgr.update_tool_request_meta( - &thread.id, - &shared_id, - "toolu_tree", - serde_json::json!({ TOOL_META_TITLE_KEY: "exploring project structure" }), - ) - .await - .unwrap(); - - let messages = mgr.list_messages(&thread.id).await.unwrap(); - assert_eq!(messages.len(), 2, "two distinct rows must be preserved"); - let text_msg = &messages[0]; - let tool_msg = &messages[1]; - assert!( - matches!(&text_msg.content[0], MessageContent::Text(_)), - "first row must remain text-only and untouched", - ); - let tr = match &tool_msg.content[0] { - MessageContent::ToolRequest(r) => r, - _ => panic!("expected tool request in second row"), - }; - assert_eq!( - tr.persisted_title(), - Some("exploring project structure"), - "title must land on the row that actually contains the tool call", - ); - } - - #[tokio::test] - async fn update_tool_request_meta_serializes_concurrent_writes_preserving_all_keys() { - // Regression for "occasional bad replay" when multiple persist tasks - // (per-tool title for tc-1, per-tool title for tc-2, chain summary on - // tc-1) race against each other for the same row's tool_meta. They - // must serialize via BEGIN IMMEDIATE and merge rather than clobber. - use crate::conversation::message::ToolRequest; - use rmcp::model::CallToolRequestParams; - - let temp = TempDir::new().unwrap(); - let mgr = fresh_thread_manager(&temp).await; - let thread = mgr.create_thread(None, None, None).await.unwrap(); - - let message = Message::new( - Role::Assistant, - chrono::Utc::now().timestamp_millis(), - vec![ - MessageContent::ToolRequest(ToolRequest { - id: "tc-1".to_string(), - tool_call: Ok(CallToolRequestParams::new("developer__shell")), - metadata: None, - tool_meta: None, - }), - MessageContent::ToolRequest(ToolRequest { - id: "tc-2".to_string(), - tool_call: Ok(CallToolRequestParams::new("developer__shell")), - metadata: None, - tool_meta: None, - }), - ], - ); - let stored = mgr - .append_message(&thread.id, None, &message) - .await - .unwrap(); - let message_id = stored.id.clone().unwrap(); - - let m1 = mgr.clone(); - let t1 = thread.id.clone(); - let mid1 = message_id.clone(); - let h1 = tokio::spawn(async move { - m1.update_tool_request_meta( - &t1, - &mid1, - "tc-1", - serde_json::json!({ TOOL_META_TITLE_KEY: "ran shell command" }), - ) - .await - .unwrap(); - }); - - let m2 = mgr.clone(); - let t2 = thread.id.clone(); - let mid2 = message_id.clone(); - let h2 = tokio::spawn(async move { - m2.update_tool_request_meta( - &t2, - &mid2, - "tc-2", - serde_json::json!({ TOOL_META_TITLE_KEY: "ran another shell command" }), - ) - .await - .unwrap(); - }); - - let m3 = mgr.clone(); - let t3 = thread.id.clone(); - let mid3 = message_id.clone(); - let h3 = tokio::spawn(async move { - m3.update_tool_request_meta( - &t3, - &mid3, - "tc-1", - serde_json::json!({ - TOOL_META_CHAIN_SUMMARY_KEY: { "summary": "inspected codebase", "count": 2 }, - }), - ) - .await - .unwrap(); - }); - - h1.await.unwrap(); - h2.await.unwrap(); - h3.await.unwrap(); - - let messages = mgr.list_messages(&thread.id).await.unwrap(); - let content = &messages[0].content; - let tc1 = match &content[0] { - MessageContent::ToolRequest(r) => r, - _ => panic!("expected tool request"), - }; - let tc2 = match &content[1] { - MessageContent::ToolRequest(r) => r, - _ => panic!("expected tool request"), - }; - - assert_eq!( - tc1.persisted_title(), - Some("ran shell command"), - "concurrent writes must not drop tc-1's title", - ); - let chain_summary = tc1 - .persisted_chain_summary() - .expect("tc-1 must keep its chain summary"); - assert_eq!(chain_summary.summary, "inspected codebase"); - assert_eq!(chain_summary.count, 2); - assert_eq!( - tc2.persisted_title(), - Some("ran another shell command"), - "concurrent writes must not drop tc-2's title", - ); - } - - #[tokio::test] - async fn update_tool_request_meta_persists_chain_summary_object() { - let temp = TempDir::new().unwrap(); - let mgr = fresh_thread_manager(&temp).await; - let thread = mgr.create_thread(None, None, None).await.unwrap(); - - let stored = mgr - .append_message( - &thread.id, - None, - &assistant_message_with_tool_request( - "tc-1", - Some(serde_json::json!({ TOOL_META_TITLE_KEY: "first step" })), - ), - ) - .await - .unwrap(); - let message_id = stored.id.clone().unwrap(); - - mgr.update_tool_request_meta( - &thread.id, - &message_id, - "tc-1", - serde_json::json!({ - TOOL_META_CHAIN_SUMMARY_KEY: { "summary": "applied dark mode polish", "count": 4 }, - }), - ) - .await - .unwrap(); - - let messages = mgr.list_messages(&thread.id).await.unwrap(); - let req = match &messages[0].content[0] { - MessageContent::ToolRequest(r) => r, - _ => panic!("expected tool request"), - }; - let chain = req - .persisted_chain_summary() - .expect("chain summary should be present"); - assert_eq!(chain.summary, "applied dark mode polish"); - assert_eq!(chain.count, 4); - assert_eq!(req.persisted_title(), Some("first step")); - } -} diff --git a/crates/goose/tests/acp_common_tests/mod.rs b/crates/goose/tests/acp_common_tests/mod.rs index 203dac4672..cad6f9336e 100644 --- a/crates/goose/tests/acp_common_tests/mod.rs +++ b/crates/goose/tests/acp_common_tests/mod.rs @@ -108,6 +108,9 @@ pub async fn run_list_sessions() { if let Some(ref mut meta) = s.meta { assert!(meta.get("createdAt").and_then(|v| v.as_str()).is_some()); meta.remove("createdAt"); + // Provider/model metadata varies by test fixture; not relevant here. + meta.remove("providerId"); + meta.remove("modelId"); } } let mut expected_meta = serde_json::Map::new(); diff --git a/crates/goose/tests/thread_message_coalescing_test.rs b/crates/goose/tests/thread_message_coalescing_test.rs deleted file mode 100644 index 012ada5a09..0000000000 --- a/crates/goose/tests/thread_message_coalescing_test.rs +++ /dev/null @@ -1,197 +0,0 @@ -use goose::conversation::message::Message; -use goose::session::session_manager::SessionStorage; -use goose::session::thread_manager::ThreadManager; -use rmcp::model::CallToolRequestParams; -use std::sync::Arc; -use tempfile::TempDir; - -async fn setup() -> (ThreadManager, TempDir) { - let tmp = TempDir::new().unwrap(); - let storage = SessionStorage::create(tmp.path()).await.unwrap(); - let tm = ThreadManager::new(Arc::new(storage)); - (tm, tmp) -} - -#[tokio::test] -async fn consecutive_text_chunks_are_coalesced() { - let (tm, _tmp) = setup().await; - let thread = tm.create_thread(None, None, None).await.unwrap(); - - // Simulate streaming: three consecutive assistant text chunks. - tm.append_message( - &thread.id, - Some("s1"), - &Message::assistant().with_text("Hello"), - ) - .await - .unwrap(); - tm.append_message( - &thread.id, - Some("s1"), - &Message::assistant().with_text(" world"), - ) - .await - .unwrap(); - tm.append_message(&thread.id, Some("s1"), &Message::assistant().with_text("!")) - .await - .unwrap(); - - let messages = tm.list_messages(&thread.id).await.unwrap(); - assert_eq!(messages.len(), 1, "should coalesce into a single row"); - assert_eq!(messages[0].as_concat_text(), "Hello world!"); -} - -#[tokio::test] -async fn role_change_prevents_coalescing() { - let (tm, _tmp) = setup().await; - let thread = tm.create_thread(None, None, None).await.unwrap(); - - tm.append_message(&thread.id, Some("s1"), &Message::user().with_text("Hi")) - .await - .unwrap(); - tm.append_message( - &thread.id, - Some("s1"), - &Message::assistant().with_text("Hey"), - ) - .await - .unwrap(); - - let messages = tm.list_messages(&thread.id).await.unwrap(); - assert_eq!(messages.len(), 2, "different roles should not coalesce"); - assert_eq!(messages[0].as_concat_text(), "Hi"); - assert_eq!(messages[1].as_concat_text(), "Hey"); -} - -#[tokio::test] -async fn non_text_content_breaks_coalescing() { - let (tm, _tmp) = setup().await; - let thread = tm.create_thread(None, None, None).await.unwrap(); - - // Text, then tool request, then more text — should be 3 rows. - tm.append_message( - &thread.id, - Some("s1"), - &Message::assistant().with_text("Let me check"), - ) - .await - .unwrap(); - - let tool_msg = Message::assistant().with_tool_request( - "call_1", - Ok(CallToolRequestParams::new("shell").with_arguments( - serde_json::json!({"command": "ls"}) - .as_object() - .unwrap() - .clone(), - )), - ); - tm.append_message(&thread.id, Some("s1"), &tool_msg) - .await - .unwrap(); - - tm.append_message( - &thread.id, - Some("s1"), - &Message::assistant().with_text("Done"), - ) - .await - .unwrap(); - - let messages = tm.list_messages(&thread.id).await.unwrap(); - assert_eq!(messages.len(), 3, "tool request should break coalescing"); - assert_eq!(messages[0].as_concat_text(), "Let me check"); - assert_eq!(messages[2].as_concat_text(), "Done"); -} - -#[tokio::test] -async fn text_after_tool_response_not_coalesced_with_tool() { - let (tm, _tmp) = setup().await; - let thread = tm.create_thread(None, None, None).await.unwrap(); - - // A tool request message (non-text) followed by text — should not coalesce. - let tool_msg = Message::assistant().with_tool_request( - "call_1", - Ok(CallToolRequestParams::new("shell").with_arguments( - serde_json::json!({"command": "ls"}) - .as_object() - .unwrap() - .clone(), - )), - ); - tm.append_message(&thread.id, Some("s1"), &tool_msg) - .await - .unwrap(); - - tm.append_message( - &thread.id, - Some("s1"), - &Message::assistant().with_text("Result"), - ) - .await - .unwrap(); - - let messages = tm.list_messages(&thread.id).await.unwrap(); - assert_eq!( - messages.len(), - 2, - "text should not coalesce with non-text predecessor" - ); -} - -#[tokio::test] -async fn empty_message_not_coalesced() { - let (tm, _tmp) = setup().await; - let thread = tm.create_thread(None, None, None).await.unwrap(); - - tm.append_message( - &thread.id, - Some("s1"), - &Message::assistant().with_text("Hello"), - ) - .await - .unwrap(); - - // An empty assistant message (no content items). - let empty = Message::assistant(); - tm.append_message(&thread.id, Some("s1"), &empty) - .await - .unwrap(); - - let messages = tm.list_messages(&thread.id).await.unwrap(); - // Empty message should be inserted as a new row (not coalesced). - assert_eq!(messages.len(), 2); - assert_eq!(messages[0].as_concat_text(), "Hello"); -} - -#[tokio::test] -async fn metadata_change_prevents_coalescing() { - let (tm, _tmp) = setup().await; - let thread = tm.create_thread(None, None, None).await.unwrap(); - - tm.append_message( - &thread.id, - Some("s1"), - &Message::assistant().with_text("Visible"), - ) - .await - .unwrap(); - tm.append_message( - &thread.id, - Some("s1"), - &Message::assistant().with_text(" hidden").agent_only(), - ) - .await - .unwrap(); - - let messages = tm.list_messages(&thread.id).await.unwrap(); - assert_eq!( - messages.len(), - 2, - "metadata boundary should break coalescing" - ); - assert!(messages[0].metadata.user_visible); - assert!(!messages[1].metadata.user_visible); - assert_eq!(messages[0].as_concat_text(), "Visible"); - assert_eq!(messages[1].as_concat_text(), " hidden"); -} diff --git a/ui/desktop/openapi.json b/ui/desktop/openapi.json index 18a3e5d858..a27e67c955 100644 --- a/ui/desktop/openapi.json +++ b/ui/desktop/openapi.json @@ -7720,6 +7720,11 @@ "format": "int32", "nullable": true }, + "archived_at": { + "type": "string", + "format": "date-time", + "nullable": true + }, "conversation": { "allOf": [ { @@ -7766,6 +7771,10 @@ "format": "int32", "nullable": true }, + "project_id": { + "type": "string", + "nullable": true + }, "provider_name": { "type": "string", "nullable": true @@ -7785,10 +7794,6 @@ "session_type": { "$ref": "#/components/schemas/SessionType" }, - "thread_id": { - "type": "string", - "nullable": true - }, "total_tokens": { "type": "integer", "format": "int32", diff --git a/ui/desktop/src/api/types.gen.ts b/ui/desktop/src/api/types.gen.ts index 3ac35db854..dee73b4e31 100644 --- a/ui/desktop/src/api/types.gen.ts +++ b/ui/desktop/src/api/types.gen.ts @@ -1256,6 +1256,7 @@ export type Session = { accumulated_input_tokens?: number | null; accumulated_output_tokens?: number | null; accumulated_total_tokens?: number | null; + archived_at?: string | null; conversation?: Conversation | null; created_at: string; extension_data: ExtensionData; @@ -1266,11 +1267,11 @@ export type Session = { model_config?: ModelConfig | null; name: string; output_tokens?: number | null; + project_id?: string | null; provider_name?: string | null; recipe?: Recipe | null; schedule_id?: string | null; session_type?: SessionType; - thread_id?: string | null; total_tokens?: number | null; updated_at: string; user_recipe_values?: { diff --git a/ui/goose2/src/features/chat/hooks/__tests__/useChat.compaction.test.ts b/ui/goose2/src/features/chat/hooks/__tests__/useChat.compaction.test.ts index c07ea0539d..b3f15ee538 100644 --- a/ui/goose2/src/features/chat/hooks/__tests__/useChat.compaction.test.ts +++ b/ui/goose2/src/features/chat/hooks/__tests__/useChat.compaction.test.ts @@ -80,11 +80,7 @@ describe("useChat compaction", () => { await result.current.compactConversation(); }); - expect(mockAcpSendMessage).toHaveBeenCalledWith( - "session-1", - "/compact", - undefined, - ); + expect(mockAcpSendMessage).toHaveBeenCalledWith("session-1", "/compact"); expect(mockAcpLoadSession).toHaveBeenCalledWith("session-1", undefined); const messages = useChatStore.getState().messagesBySession["session-1"]; @@ -121,35 +117,6 @@ describe("useChat compaction", () => { ); }); - it("prepares and compacts the override persona session", async () => { - let preparedPersonaId: string | undefined; - const ensurePrepared = vi.fn(async (personaId?: string) => { - preparedPersonaId = personaId; - return undefined; - }); - - const { result } = renderHook(() => - useChat( - "session-1", - undefined, - undefined, - { id: "persona-b", name: "Persona B" }, - { ensurePrepared }, - ), - ); - - await act(async () => { - await result.current.compactConversation({ id: "persona-a" }); - }); - - expect(ensurePrepared).toHaveBeenCalledWith("persona-a"); - expect(mockAcpSendMessage).toHaveBeenCalledWith("session-1", "/compact", { - personaId: "persona-a", - }); - expect(mockAcpLoadSession).toHaveBeenCalledWith("session-1", undefined); - expect(preparedPersonaId).toBe("persona-a"); - }); - it("blocks new sends while compaction is in flight", async () => { const compactDeferred = createDeferredPromise(); mockAcpSendMessage.mockImplementation( @@ -174,11 +141,7 @@ describe("useChat compaction", () => { }); expect(mockAcpSendMessage).toHaveBeenCalledTimes(1); - expect(mockAcpSendMessage).toHaveBeenCalledWith( - "session-1", - "/compact", - undefined, - ); + expect(mockAcpSendMessage).toHaveBeenCalledWith("session-1", "/compact"); expect( useChatStore.getState().messagesBySession["session-1"], ).toBeUndefined(); @@ -214,11 +177,7 @@ describe("useChat compaction", () => { }); expect(mockAcpSendMessage).toHaveBeenCalledTimes(1); - expect(mockAcpSendMessage).toHaveBeenCalledWith( - "session-1", - "/compact", - undefined, - ); + expect(mockAcpSendMessage).toHaveBeenCalledWith("session-1", "/compact"); expect(mockAcpLoadSession).not.toHaveBeenCalled(); expect( useChatStore.getState().getSessionRuntime("session-1").chatState, diff --git a/ui/goose2/src/features/chat/hooks/__tests__/useChat.personaPreparation.test.ts b/ui/goose2/src/features/chat/hooks/__tests__/useChat.personaPreparation.test.ts deleted file mode 100644 index 70edd53968..0000000000 --- a/ui/goose2/src/features/chat/hooks/__tests__/useChat.personaPreparation.test.ts +++ /dev/null @@ -1,96 +0,0 @@ -import { act, renderHook } from "@testing-library/react"; -import { beforeEach, describe, expect, it, vi } from "vitest"; -import { useAgentStore } from "@/features/agents/stores/agentStore"; -import { useChatStore } from "../../stores/chatStore"; -import { useChatSessionStore } from "../../stores/chatSessionStore"; -import { clearReplayBuffer } from "../replayBuffer"; - -const mockAcpSendMessage = vi.fn(); -const mockAcpCancelSession = vi.fn(); -const mockAcpLoadSession = vi.fn(); - -vi.mock("@/shared/api/acp", () => ({ - acpSendMessage: (...args: unknown[]) => mockAcpSendMessage(...args), - acpCancelSession: (...args: unknown[]) => mockAcpCancelSession(...args), - acpLoadSession: (...args: unknown[]) => mockAcpLoadSession(...args), -})); - -import { useChat } from "../useChat"; - -describe("useChat persona preparation", () => { - beforeEach(() => { - mockAcpSendMessage.mockReset(); - mockAcpCancelSession.mockReset(); - mockAcpLoadSession.mockReset(); - clearReplayBuffer("session-1"); - useChatStore.setState({ - messagesBySession: {}, - sessionStateById: {}, - activeSessionId: null, - isConnected: true, - }); - useChatSessionStore.setState({ - sessions: [], - activeSessionId: null, - isLoading: false, - contextPanelOpenBySession: {}, - activeWorkspaceBySession: {}, - }); - useAgentStore.setState({ - personas: [ - { - id: "persona-a", - displayName: "Persona A", - systemPrompt: "", - isBuiltin: false, - createdAt: "", - updatedAt: "", - }, - { - id: "persona-b", - displayName: "Persona B", - systemPrompt: "", - isBuiltin: false, - createdAt: "", - updatedAt: "", - }, - ], - personasLoading: false, - agents: [], - agentsLoading: false, - activeAgentId: null, - isLoading: false, - personaEditorOpen: false, - editingPersona: null, - }); - mockAcpSendMessage.mockResolvedValue(undefined); - mockAcpCancelSession.mockResolvedValue(true); - mockAcpLoadSession.mockResolvedValue(undefined); - }); - - it("prepares the override persona before prompting", async () => { - const ensurePrepared = vi.fn().mockResolvedValue(undefined); - - const { result } = renderHook(() => - useChat( - "session-1", - undefined, - undefined, - { id: "persona-a", name: "Persona A" }, - { ensurePrepared }, - ), - ); - - await act(async () => { - await result.current.sendMessage("Hello", { id: "persona-b" }); - }); - - expect(ensurePrepared).toHaveBeenCalledWith("persona-b"); - expect(mockAcpSendMessage).toHaveBeenCalledWith("session-1", "Hello", { - systemPrompt: undefined, - personaId: "persona-b", - personaName: "Persona B", - images: undefined, - }); - }); -}); diff --git a/ui/goose2/src/features/chat/hooks/useChat.ts b/ui/goose2/src/features/chat/hooks/useChat.ts index de83543223..948b321170 100644 --- a/ui/goose2/src/features/chat/hooks/useChat.ts +++ b/ui/goose2/src/features/chat/hooks/useChat.ts @@ -268,8 +268,7 @@ export function useChat( ...(sendOptions?.assistantPrompt ? { assistantPrompt: sendOptions.assistantPrompt } : {}), - personaId: effectivePersonaInfo?.id, - personaName: effectivePersonaInfo?.name, + images: images?.map( (img) => [img.base64, img.mimeType] as [string, string], ), @@ -415,10 +414,7 @@ export function useChat( clearReplayBuffer(sessionId); try { - const sendOptions = effectivePersonaInfo?.id - ? { personaId: effectivePersonaInfo.id } - : undefined; - await acpSendMessage(sessionId, MANUAL_COMPACT_TRIGGER, sendOptions); + await acpSendMessage(sessionId, MANUAL_COMPACT_TRIGGER); // Command responses are streamed via prompt notifications, but the ACP // layer does not currently forward history replacement events. Drop those diff --git a/ui/goose2/src/features/chat/hooks/useChatSessionController.ts b/ui/goose2/src/features/chat/hooks/useChatSessionController.ts index 0e48b100ef..c5f804dfc8 100644 --- a/ui/goose2/src/features/chat/hooks/useChatSessionController.ts +++ b/ui/goose2/src/features/chat/hooks/useChatSessionController.ts @@ -108,7 +108,7 @@ export function useChatSessionController({ const selectedPersonaId = pendingPersonaId !== undefined ? pendingPersonaId - : (session?.personaId ?? null); + : (session?.agentId ?? null); const selectedPersona = personas.find( (persona) => persona.id === selectedPersonaId, ); @@ -378,7 +378,7 @@ export function useChatSessionController({ } useChatSessionStore .getState() - .patchSession(sessionId, { personaId: personaId ?? undefined }); + .patchSession(sessionId, { agentId: personaId ?? undefined }); }, [ handleProviderChange, @@ -399,7 +399,7 @@ export function useChatSessionController({ if (sessionId) { useChatSessionStore .getState() - .patchSession(sessionId, { personaId: undefined }); + .patchSession(sessionId, { agentId: undefined }); } else { setPendingPersonaId(undefined); } @@ -595,7 +595,7 @@ export function useChatSessionController({ const nextPersonaId = pendingPersonaId !== undefined ? (pendingPersonaId ?? undefined) - : session?.personaId; + : session?.agentId; const nextProjectId = pendingProjectId !== undefined ? pendingProjectId @@ -610,10 +610,10 @@ export function useChatSessionController({ const patch: { providerId?: string; - personaId?: string | undefined; + agentId?: string | undefined; + projectId?: string | null; modelId?: string | undefined; modelName?: string | undefined; - projectId?: string | null; } = {}; if (hasPendingProvider) { @@ -622,7 +622,7 @@ export function useChatSessionController({ patch.modelName = undefined; } if (hasPendingPersona) { - patch.personaId = nextPersonaId; + patch.agentId = nextPersonaId; } if (hasPendingProject) { patch.projectId = nextProjectId ?? null; @@ -690,7 +690,7 @@ export function useChatSessionController({ pendingQueuedMessage, prepareCurrentSession, selectedProvider, - session?.personaId, + session?.agentId, session?.projectId, sessionId, ]); diff --git a/ui/goose2/src/features/chat/stores/__tests__/chatSessionStore.test.ts b/ui/goose2/src/features/chat/stores/__tests__/chatSessionStore.test.ts index d7f708eb22..c9eef9c10e 100644 --- a/ui/goose2/src/features/chat/stores/__tests__/chatSessionStore.test.ts +++ b/ui/goose2/src/features/chat/stores/__tests__/chatSessionStore.test.ts @@ -61,7 +61,7 @@ describe("chatSessionStore", () => { title: "New Chat", providerId: "openai", projectId: "project-1", - personaId: "persona-1", + agentId: "persona-1", modelId: "gpt-4.1", modelName: "GPT-4.1", workingDir: "/tmp/project", @@ -72,7 +72,6 @@ describe("chatSessionStore", () => { "/tmp/project", { projectId: "project-1", - personaId: "persona-1", modelId: "gpt-4.1", }, ); @@ -81,7 +80,7 @@ describe("chatSessionStore", () => { title: "New Chat", projectId: "project-1", providerId: "openai", - personaId: "persona-1", + agentId: "persona-1", modelId: "gpt-4.1", modelName: "GPT-4.1", workingDir: "/tmp/project", @@ -146,7 +145,6 @@ describe("chatSessionStore", () => { workingDir: "/tmp/project-123", projectId: "project-123", providerId: "anthropic", - personaId: "persona-1", modelId: "claude-sonnet-4", }, ]); @@ -157,7 +155,6 @@ describe("chatSessionStore", () => { expect(session.title).toBe("Renamed Chat"); expect(session.projectId).toBe("project-123"); expect(session.providerId).toBe("anthropic"); - expect(session.personaId).toBe("persona-1"); expect(session.createdAt).toBe("2026-03-31"); expect(session.updatedAt).toBe("2026-04-02"); expect(session.messageCount).toBe(7); diff --git a/ui/goose2/src/features/chat/stores/chatSessionStore.ts b/ui/goose2/src/features/chat/stores/chatSessionStore.ts index d49e2b1dbc..abc0986c6e 100644 --- a/ui/goose2/src/features/chat/stores/chatSessionStore.ts +++ b/ui/goose2/src/features/chat/stores/chatSessionStore.ts @@ -19,7 +19,7 @@ export interface ChatSession { title: string; projectId?: string | null; providerId?: string; - personaId?: string; + agentId?: string; modelId?: string; modelName?: string; workingDir?: string | null; @@ -66,7 +66,7 @@ interface CreateSessionOpts { title?: string; projectId?: string; providerId?: string; - personaId?: string; + agentId?: string; workingDir?: string; modelId?: string; modelName?: string; @@ -100,7 +100,6 @@ function acpSessionToChatSession(session: AcpSessionInfo): ChatSession { title: normalizeAcpTitle(session.title) ?? "Untitled", projectId: session.projectId ?? undefined, providerId: session.providerId ?? undefined, - personaId: session.personaId ?? undefined, modelId: session.modelId ?? undefined, workingDir: session.workingDir ?? undefined, createdAt: session.createdAt ?? session.updatedAt ?? now, @@ -123,7 +122,6 @@ export function sessionToChatSession(session: Session): ChatSession { title: session.title, projectId: session.projectId, providerId: session.providerId, - personaId: session.personaId, modelId: session.modelId, modelName: session.modelName, workingDir: session.workingDir, @@ -150,7 +148,6 @@ export const useChatSessionStore = create((set, get) => ({ const now = new Date().toISOString(); const providerId = opts.providerId ?? "goose"; const { sessionId } = await acpCreateSession(providerId, opts.workingDir, { - personaId: opts.personaId, modelId: opts.modelId, projectId: opts.projectId, }); @@ -159,7 +156,7 @@ export const useChatSessionStore = create((set, get) => ({ title: opts.title ?? DEFAULT_CHAT_TITLE, projectId: opts.projectId, providerId, - personaId: opts.personaId, + agentId: opts.agentId, modelId: opts.modelId, modelName: opts.modelName, workingDir: opts.workingDir, diff --git a/ui/goose2/src/features/sessions/lib/buildSessionSearchResults.test.ts b/ui/goose2/src/features/sessions/lib/buildSessionSearchResults.test.ts index 1a4e4d14f6..1effb8763b 100644 --- a/ui/goose2/src/features/sessions/lib/buildSessionSearchResults.test.ts +++ b/ui/goose2/src/features/sessions/lib/buildSessionSearchResults.test.ts @@ -30,7 +30,7 @@ describe("buildSessionSearchResults", () => { makeSession({ id: "session-2", title: "Builder notes", - personaId: "persona-1", + agentId: "persona-1", updatedAt: "2026-04-09T12:00:00Z", }), ]; diff --git a/ui/goose2/src/features/sessions/lib/filterSessions.test.ts b/ui/goose2/src/features/sessions/lib/filterSessions.test.ts index 23f748c4e7..f2a0ae9550 100644 --- a/ui/goose2/src/features/sessions/lib/filterSessions.test.ts +++ b/ui/goose2/src/features/sessions/lib/filterSessions.test.ts @@ -22,7 +22,7 @@ function makeSession( describe("filterSessions", () => { const sessions: ChatSession[] = [ - makeSession({ id: "1", title: "Fix sidebar bug", personaId: "p1" }), + makeSession({ id: "1", title: "Fix sidebar bug", agentId: "p1" }), makeSession({ id: "2", title: "Add pagination", diff --git a/ui/goose2/src/features/sessions/lib/filterSessions.ts b/ui/goose2/src/features/sessions/lib/filterSessions.ts index 84e5d3f685..ac2da54f77 100644 --- a/ui/goose2/src/features/sessions/lib/filterSessions.ts +++ b/ui/goose2/src/features/sessions/lib/filterSessions.ts @@ -23,8 +23,8 @@ function buildSearchableString( parts.push(session.title); } - if (session.personaId) { - const name = resolvers.getPersonaName(session.personaId); + if (session.agentId) { + const name = resolvers.getPersonaName(session.agentId); if (name) parts.push(name); } diff --git a/ui/goose2/src/features/sessions/ui/SessionHistoryView.tsx b/ui/goose2/src/features/sessions/ui/SessionHistoryView.tsx index d9d2197bb6..e5646bf299 100644 --- a/ui/goose2/src/features/sessions/ui/SessionHistoryView.tsx +++ b/ui/goose2/src/features/sessions/ui/SessionHistoryView.tsx @@ -236,8 +236,8 @@ export function SessionHistoryView({ title={result.session.title} updatedAt={result.session.updatedAt} personaName={ - result.session.personaId - ? getPersonaName(result.session.personaId) + result.session.agentId + ? getPersonaName(result.session.agentId) : undefined } projectName={ @@ -299,8 +299,8 @@ export function SessionHistoryView({ title={session.title} updatedAt={session.updatedAt} personaName={ - session.personaId - ? getPersonaName(session.personaId) + session.agentId + ? getPersonaName(session.agentId) : undefined } projectName={ diff --git a/ui/goose2/src/features/sidebar/ui/SidebarSearchResults.tsx b/ui/goose2/src/features/sidebar/ui/SidebarSearchResults.tsx index d28bb9e90d..d5c726325f 100644 --- a/ui/goose2/src/features/sidebar/ui/SidebarSearchResults.tsx +++ b/ui/goose2/src/features/sidebar/ui/SidebarSearchResults.tsx @@ -41,8 +41,8 @@ export function SidebarSearchResults({ session.title, t("common:session.defaultTitle"), ); - const personaName = session.personaId - ? getPersonaName(session.personaId) + const personaName = session.agentId + ? getPersonaName(session.agentId) : undefined; const projectName = session.projectId ? getProjectName(session.projectId) diff --git a/ui/goose2/src/shared/api/__tests__/acp.test.ts b/ui/goose2/src/shared/api/__tests__/acp.test.ts index e428afd930..087bee463d 100644 --- a/ui/goose2/src/shared/api/__tests__/acp.test.ts +++ b/ui/goose2/src/shared/api/__tests__/acp.test.ts @@ -69,7 +69,6 @@ describe("acpCreateSession", () => { await expect( acpCreateSession("openai", "/tmp/project", { projectId: "project-1", - personaId: "persona-1", modelId: "gpt-4.1", }), ).resolves.toEqual({ sessionId: "acp-session-1" }); @@ -78,7 +77,6 @@ describe("acpCreateSession", () => { "/tmp/project", "openai", "project-1", - "persona-1", ); expect(mockLoadSession).not.toHaveBeenCalled(); expect(mockSetProvider).toHaveBeenCalledWith("acp-session-1", "openai"); diff --git a/ui/goose2/src/shared/api/acp.ts b/ui/goose2/src/shared/api/acp.ts index aeb3a8df5e..f0bafaac7e 100644 --- a/ui/goose2/src/shared/api/acp.ts +++ b/ui/goose2/src/shared/api/acp.ts @@ -21,14 +21,11 @@ export interface AcpProvider { export interface AcpSendMessageOptions { systemPrompt?: string; assistantPrompt?: string; - personaId?: string; - personaName?: string; /** Image attachments as [base64Data, mimeType] pairs. */ images?: [string, string][]; } export interface AcpCreateSessionOptions { - personaId?: string; projectId?: string; modelId?: string | null; } @@ -79,7 +76,7 @@ export async function acpSendMessage( prompt: string, options: AcpSendMessageOptions = {}, ): Promise { - const { systemPrompt, assistantPrompt, personaId, images } = options; + const { systemPrompt, assistantPrompt, images } = options; const sid = sessionId.slice(0, 8); const tStart = performance.now(); @@ -116,14 +113,8 @@ export async function acpSendMessage( `[perf:send] ${sid} acpSendMessage → prompt(len=${prompt.length}, imgs=${images?.length ?? 0})`, ); const tPrompt = performance.now(); - const meta: Record = {}; - if (personaId) meta.personaId = personaId; try { - await directAcp.prompt( - sessionId, - content, - Object.keys(meta).length > 0 ? meta : undefined, - ); + await directAcp.prompt(sessionId, content); const tDone = performance.now(); perfLog( `[perf:send] ${sid} prompt() resolved in ${(tDone - tPrompt).toFixed(1)}ms (total acpSendMessage ${(tDone - tStart).toFixed(1)}ms)`, @@ -159,7 +150,6 @@ export async function acpCreateSession( workingDir, providerId, options.projectId, - options.personaId, ); const sessionId = response.sessionId; await directAcp.setProvider(sessionId, providerId); diff --git a/ui/goose2/src/shared/api/acpApi.ts b/ui/goose2/src/shared/api/acpApi.ts index a797af32b8..b94d2aef32 100644 --- a/ui/goose2/src/shared/api/acpApi.ts +++ b/ui/goose2/src/shared/api/acpApi.ts @@ -26,7 +26,6 @@ export interface AcpSessionInfo { projectId?: string | null; providerId: string | null; modelId: string | null; - personaId: string | null; } export const DEPRECATED_PROVIDER_IDS = new Set([ @@ -83,7 +82,6 @@ export async function listSessions(): Promise { projectId: (info._meta?.projectId as string) ?? null, providerId: (info._meta?.providerId as string) ?? null, modelId: (info._meta?.modelId as string) ?? null, - personaId: (info._meta?.personaId as string) ?? null, })); } @@ -118,7 +116,6 @@ export async function forkSession(sessionId: string): Promise { projectId: (response._meta?.projectId as string) ?? null, providerId: (response._meta?.providerId as string) ?? null, modelId: (response._meta?.modelId as string) ?? null, - personaId: (response._meta?.personaId as string) ?? null, }; } @@ -207,7 +204,6 @@ export async function newSession( workingDir: string, providerId?: string, projectId?: string, - personaId?: string, ): Promise { const tClient = performance.now(); const client = await getClient(); @@ -216,11 +212,10 @@ export async function newSession( mcpServers: [], }; - const meta: Record = {}; + const meta: Record = { client: "goose" }; if (providerId) meta.provider = providerId; if (projectId) meta.projectId = projectId; - if (personaId) meta.personaId = personaId; - if (Object.keys(meta).length > 0) request._meta = meta; + request._meta = meta; const tCall = performance.now(); const response = await client.newSession(request); diff --git a/ui/goose2/src/shared/types/chat.ts b/ui/goose2/src/shared/types/chat.ts index 6205977d29..cb6e824955 100644 --- a/ui/goose2/src/shared/types/chat.ts +++ b/ui/goose2/src/shared/types/chat.ts @@ -54,7 +54,6 @@ export interface Session { title: string; projectId?: string | null; providerId?: string; - personaId?: string; modelId?: string; modelName?: string; workingDir?: string | null; diff --git a/ui/goose2/tests/e2e/drafts.spec.ts b/ui/goose2/tests/e2e/drafts.spec.ts index 57e8ab2962..a0263d91d6 100644 --- a/ui/goose2/tests/e2e/drafts.spec.ts +++ b/ui/goose2/tests/e2e/drafts.spec.ts @@ -8,11 +8,16 @@ async function clickNewChatInProject( name: projectName, exact: true, }); - await projectButton.hover(); - await projectButton - .locator("xpath=..") - .getByTitle("New chat in project") - .click(); + // The "New chat" button uses group-hover:visible and is invisible by default. + // Headless Playwright on Linux doesn't reliably trigger CSS :hover, + // so we force the button visible via JS before clicking. + const row = projectButton.locator("xpath=.."); + const newChatBtn = row.getByTitle("New chat in project"); + await newChatBtn.evaluate((el) => { + el.style.visibility = "visible"; + el.style.opacity = "1"; + }); + await newChatBtn.click(); } test.describe("Draft persistence", () => { diff --git a/ui/goose2/tests/e2e/fixtures/tauri-mock.ts b/ui/goose2/tests/e2e/fixtures/tauri-mock.ts index e028fba9b2..108e507574 100644 --- a/ui/goose2/tests/e2e/fixtures/tauri-mock.ts +++ b/ui/goose2/tests/e2e/fixtures/tauri-mock.ts @@ -124,6 +124,27 @@ export function buildInitScript(options?: { supportingFiles: [], }); + const projectToSourceEntry = (p) => ({ + type: "project", + name: p.id ?? p.name?.toLowerCase(), + description: p.description ?? "", + content: p.prompt ?? "", + path: "/mock/.agents/projects/" + (p.id ?? p.name?.toLowerCase()), + global: true, + supportingFiles: [], + properties: { + title: p.name, + icon: p.icon ?? "", + color: p.color ?? "", + preferredProvider: p.preferredProvider ?? null, + preferredModel: p.preferredModel ?? null, + workingDirs: p.workingDirs ?? [], + useWorktrees: p.useWorktrees ?? false, + order: p.order ?? 0, + archivedAt: null, + }, + }); + function nowIso() { return new Date().toISOString(); } @@ -240,8 +261,13 @@ export function buildInitScript(options?: { case "_goose/working_dir/update": case "goose/working_dir/update": return jsonRpcResult(message.id, {}); - case "_goose/sources/list": + case "_goose/sources/list": { + const sourceType = message.params?.type; + if (sourceType === "project") { + return jsonRpcResult(message.id, { sources: PROJECTS.map(projectToSourceEntry) }); + } return jsonRpcResult(message.id, { sources: SKILLS.map(skillToSourceEntry) }); + } case "_goose/sources/create": return jsonRpcResult(message.id, { source: { diff --git a/ui/goose2/tests/e2e/skills.spec.ts b/ui/goose2/tests/e2e/skills.spec.ts index b8cb068173..5eba52d4fc 100644 --- a/ui/goose2/tests/e2e/skills.spec.ts +++ b/ui/goose2/tests/e2e/skills.spec.ts @@ -52,8 +52,12 @@ test.describe("Skills view", () => { await page.getByPlaceholder("Search skills").fill("review"); - await expect(page.getByText("code-review")).toBeVisible(); - await expect(page.getByText("test-writer")).not.toBeVisible(); + await expect( + page.getByRole("button", { name: "Open code-review details" }), + ).toBeVisible(); + await expect( + page.getByRole("button", { name: "Open test-writer details" }), + ).not.toBeVisible(); }); test("project filtering isolates project skills", async ({