diff --git a/crates/goose/src/acp/server.rs b/crates/goose/src/acp/server.rs index 783ef7dd7f9c..cf7566c94eec 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -34,7 +34,6 @@ use crate::providers::inventory::{ ProviderInventoryEntry, ProviderInventoryService, RefreshJobPlan, RefreshPlan, RefreshSkipReason, }; -use crate::session::session_manager::{SessionListCursor, SessionType}; use crate::session::{ EnabledExtensionsState, ExtensionData, ExtensionState, Session, SessionManager, }; @@ -50,7 +49,7 @@ use agent_client_protocol::schema::{ McpCapabilities, McpServer, Meta, NewSessionRequest, NewSessionResponse, PermissionOption, PermissionOptionKind, PromptCapabilities, PromptRequest, PromptResponse, RequestPermissionOutcome, RequestPermissionRequest, ResourceLink, SessionCapabilities, - SessionCloseCapabilities, SessionConfigOption, SessionId, SessionInfo, SessionInfoUpdate, + SessionCloseCapabilities, SessionConfigOption, SessionId, SessionInfoUpdate, SessionListCapabilities, SessionNotification, SessionUpdate, SetSessionConfigOptionRequest, SetSessionConfigOptionResponse, SetSessionModeRequest, SetSessionModeResponse, SetSessionModelRequest, SetSessionModelResponse, StopReason, TextContent, TextResourceContents, @@ -63,7 +62,6 @@ use agent_client_protocol::{ Responder, }; use anyhow::Result; -use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine as _}; use fs_err as fs; use futures::future::BoxFuture; use futures::stream::{self, StreamExt}; @@ -71,8 +69,7 @@ use futures::FutureExt; use rmcp::model::{ AnnotateAble, CallToolResult, RawContent, RawTextContent, ResourceContents, Role, }; -use serde::{Deserialize, Serialize}; -use sha2::{Digest, Sha256}; +use serde::Deserialize; use std::collections::{HashMap, HashSet}; use std::panic::AssertUnwindSafe; use std::path::{Path, PathBuf}; @@ -89,6 +86,7 @@ mod dictation; mod dispatch; mod extensions; mod fork_session; +mod list_sessions; mod load_session; mod manage_sessions; mod new_session; @@ -109,10 +107,6 @@ pub type AcpProviderFactory = Arc< + Sync, >; -const SESSION_LIST_PAGE_SIZE: usize = 50; -const ACP_SESSION_LIST_TYPES: [SessionType; 3] = - [SessionType::User, SessionType::Scheduled, SessionType::Acp]; - /// Convenience conversions from any `Display` error into an `agent_client_protocol::Error`. /// /// Replaces the repetitive `.internal_err()` @@ -226,94 +220,6 @@ pub(super) fn sid_short(id: &str) -> String { id.chars().take(8).collect() } -#[derive(Debug, Serialize, Deserialize)] -struct SessionListCursorToken { - updated_at: chrono::DateTime, - // Goose stores updated_at with second precision in common write paths, so the - // cursor needs the full (updated_at, id) sort key to avoid skipping tied rows. - session_id: String, - filter_hash: String, -} - -#[derive(Debug, Serialize)] -#[serde(rename_all = "camelCase")] -struct SessionListCursorFilters { - cwd: Option, - session_types: Vec, - non_empty: bool, -} - -fn invalid_session_list_cursor(message: &'static str) -> agent_client_protocol::Error { - agent_client_protocol::Error::invalid_params().data(message) -} - -// bind cursors to the effective filters so they cannot be reused for a different list. -fn session_list_filter_hash( - cwd: Option<&std::path::Path>, - session_types: &[SessionType], -) -> Result { - let mut session_type_names = session_types - .iter() - .map(ToString::to_string) - .collect::>(); - session_type_names.sort(); - let filters = SessionListCursorFilters { - cwd: cwd.map(|path| path.to_string_lossy().to_string()), - session_types: session_type_names, - non_empty: true, - }; - let bytes = - serde_json::to_vec(&filters).internal_err_ctx("Failed to encode session list filters")?; - Ok(URL_SAFE_NO_PAD.encode(Sha256::digest(bytes))) -} - -fn decode_session_list_cursor( - cursor: Option<&str>, - cwd: Option<&std::path::Path>, - session_types: &[SessionType], -) -> Result, agent_client_protocol::Error> { - let Some(cursor) = cursor else { - return Ok(None); - }; - - let bytes = URL_SAFE_NO_PAD - .decode(cursor) - .map_err(|_| invalid_session_list_cursor("malformed session list cursor"))?; - let token: SessionListCursorToken = serde_json::from_slice(&bytes) - .map_err(|_| invalid_session_list_cursor("malformed session list cursor"))?; - - if token.session_id.is_empty() || token.filter_hash.is_empty() { - return Err(invalid_session_list_cursor("malformed session list cursor")); - } - - let expected_filter_hash = session_list_filter_hash(cwd, session_types)?; - if token.filter_hash != expected_filter_hash { - return Err(invalid_session_list_cursor( - "session list cursor does not match filters", - )); - } - - Ok(Some(SessionListCursor { - updated_at: token.updated_at, - session_id: token.session_id, - })) -} - -fn encode_session_list_cursor( - cursor: &SessionListCursor, - cwd: Option<&std::path::Path>, - session_types: &[SessionType], -) -> Result { - let token = SessionListCursorToken { - updated_at: cursor.updated_at, - session_id: cursor.session_id.clone(), - filter_hash: session_list_filter_hash(cwd, session_types)?, - }; - let bytes = - serde_json::to_vec(&token).internal_err_ctx("Failed to encode session list cursor")?; - Ok(URL_SAFE_NO_PAD.encode(bytes)) -} - pub(super) fn session_meta(session: &Session) -> serde_json::Map { let mut meta = serde_json::Map::new(); meta.insert( @@ -360,10 +266,22 @@ pub(super) fn session_meta(session: &Session) -> serde_json::Map, key: &str) -> Option { - meta.and_then(|m| m.get(key)) - .and_then(|v| v.as_str()) - .map(ToString::to_string) +fn meta_string( + meta: Option<&Meta>, + key: &str, +) -> Result, agent_client_protocol::Error> { + let Some(value) = meta.and_then(|m| m.get(key)) else { + return Ok(None); + }; + if value.is_null() { + return Ok(None); + } + let Some(value) = value.as_str() else { + return Err( + agent_client_protocol::Error::invalid_params().data(format!("{key} must be a string")) + ); + }; + Ok(Some(value.to_string())) } fn spawn_session_name_update_notifier( @@ -2817,55 +2735,6 @@ impl GooseAcpAgent { Ok(()) } - async fn on_list_sessions( - &self, - req: ListSessionsRequest, - ) -> Result { - if let Some(cwd) = req.cwd.as_deref() { - if !cwd.is_absolute() { - return Err(agent_client_protocol::Error::invalid_params() - .data("cwd must be an absolute path")); - } - } - - let cwd = req.cwd.as_deref(); - let cursor = - decode_session_list_cursor(req.cursor.as_deref(), cwd, &ACP_SESSION_LIST_TYPES)?; - - // ACP clients see their own (Acp) sessions plus legacy User/Scheduled ones. - let page = self - .session_manager - .list_nonempty_sessions_by_types_paged( - &ACP_SESSION_LIST_TYPES, - cwd, - cursor.as_ref(), - SESSION_LIST_PAGE_SIZE, - ) - .await - .internal_err()?; - let session_infos: Vec = page - .sessions - .into_iter() - .map(|s| { - let meta = session_meta(&s); - let title = s.display_title(); - let mut info = SessionInfo::new(SessionId::new(s.id), s.working_dir) - .updated_at(s.updated_at.to_rfc3339()) - .meta(meta); - if let Some(t) = title { - info = info.title(t); - } - info - }) - .collect(); - let next_cursor = page - .next_cursor - .as_ref() - .map(|cursor| encode_session_list_cursor(cursor, cwd, &ACP_SESSION_LIST_TYPES)) - .transpose()?; - Ok(ListSessionsResponse::new(session_infos).next_cursor(next_cursor)) - } - async fn on_fork_session( &self, cx: &ConnectionTo, @@ -2944,6 +2813,7 @@ pub async fn run(builtins: Vec) -> Result<()> { mod tests { use super::*; use crate::conversation::message::{ToolRequest, ToolResponse}; + use crate::session::session_manager::SessionType; use agent_client_protocol::schema::{ EnvVariable, HttpHeader, McpServer, McpServerHttp, McpServerSse, McpServerStdio, PermissionOptionId, ResourceLink, SelectedPermissionOutcome, diff --git a/crates/goose/src/acp/server/list_sessions.rs b/crates/goose/src/acp/server/list_sessions.rs new file mode 100644 index 000000000000..eea716af1124 --- /dev/null +++ b/crates/goose/src/acp/server/list_sessions.rs @@ -0,0 +1,207 @@ +use super::{meta_string, session_meta, GooseAcpAgent, ResultExt}; +use crate::session::session_manager::{ + SessionListCursor, SessionListFilters, SessionListPageQuery, SessionType, +}; +use agent_client_protocol::schema::{ + ListSessionsRequest, ListSessionsResponse, Meta, SessionId, SessionInfo, +}; +use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine as _}; +use serde::{Deserialize, Serialize}; +use sha2::{Digest, Sha256}; + +const SESSION_LIST_PAGE_SIZE: usize = 50; +const ACP_SESSION_LIST_TYPES: [SessionType; 3] = + [SessionType::User, SessionType::Scheduled, SessionType::Acp]; + +#[derive(Debug, Serialize, Deserialize)] +struct SessionListCursorToken { + updated_at: chrono::DateTime, + // Goose stores updated_at with second precision in common write paths, so the + // cursor needs the full (updated_at, id) sort key to avoid skipping tied rows. + session_id: String, + filter_hash: String, +} + +#[derive(Debug, Serialize)] +#[serde(rename_all = "camelCase")] +struct SessionListCursorFilters { + cwd: Option, + session_types: Vec, + keyword: Option, + only_sessions_with_messages: bool, +} + +fn invalid_session_list_cursor(message: &'static str) -> agent_client_protocol::Error { + agent_client_protocol::Error::invalid_params().data(message) +} + +fn session_keyword_from_meta( + meta: Option<&Meta>, +) -> Result, agent_client_protocol::Error> { + Ok(meta_string(meta, "query")? + .map(|keyword| keyword.trim().to_string()) + .filter(|keyword| !keyword.is_empty())) +} + +fn session_types_from_meta( + meta: Option<&Meta>, +) -> Result, agent_client_protocol::Error> { + let Some(value) = meta.and_then(|meta| meta.get("types")) else { + return Ok(ACP_SESSION_LIST_TYPES.to_vec()); + }; + if value.is_null() { + return Ok(ACP_SESSION_LIST_TYPES.to_vec()); + } + + let session_types = + serde_json::from_value::>(value.clone()).map_err(|_| { + agent_client_protocol::Error::invalid_params() + .data("types must be an array of session type strings") + })?; + if session_types.is_empty() { + Ok(ACP_SESSION_LIST_TYPES.to_vec()) + } else { + if session_types + .iter() + .any(|session_type| !ACP_SESSION_LIST_TYPES.contains(session_type)) + { + return Err(agent_client_protocol::Error::invalid_params() + .data("types may only include user, scheduled, or acp")); + } + Ok(session_types) + } +} + +// bind cursors to the effective filters so they cannot be reused for a different list. +fn session_list_filter_hash( + cwd: Option<&std::path::Path>, + session_types: &[SessionType], + keyword: Option<&str>, +) -> Result { + let mut session_type_names = session_types + .iter() + .map(ToString::to_string) + .collect::>(); + session_type_names.sort(); + let filters = SessionListCursorFilters { + cwd: cwd.map(|path| path.to_string_lossy().to_string()), + session_types: session_type_names, + keyword: keyword.map(ToString::to_string), + only_sessions_with_messages: true, + }; + let bytes = + serde_json::to_vec(&filters).internal_err_ctx("Failed to encode session list filters")?; + Ok(URL_SAFE_NO_PAD.encode(Sha256::digest(bytes))) +} + +fn decode_session_list_cursor( + cursor: Option<&str>, + cwd: Option<&std::path::Path>, + session_types: &[SessionType], + keyword: Option<&str>, +) -> Result, agent_client_protocol::Error> { + let Some(cursor) = cursor else { + return Ok(None); + }; + + let bytes = URL_SAFE_NO_PAD + .decode(cursor) + .map_err(|_| invalid_session_list_cursor("malformed session list cursor"))?; + let token: SessionListCursorToken = serde_json::from_slice(&bytes) + .map_err(|_| invalid_session_list_cursor("malformed session list cursor"))?; + + if token.session_id.is_empty() || token.filter_hash.is_empty() { + return Err(invalid_session_list_cursor("malformed session list cursor")); + } + + let expected_filter_hash = session_list_filter_hash(cwd, session_types, keyword)?; + if token.filter_hash != expected_filter_hash { + return Err(invalid_session_list_cursor( + "session list cursor does not match filters", + )); + } + + Ok(Some(SessionListCursor { + updated_at: token.updated_at, + session_id: token.session_id, + })) +} + +fn encode_session_list_cursor( + cursor: &SessionListCursor, + cwd: Option<&std::path::Path>, + session_types: &[SessionType], + keyword: Option<&str>, +) -> Result { + let token = SessionListCursorToken { + updated_at: cursor.updated_at, + session_id: cursor.session_id.clone(), + filter_hash: session_list_filter_hash(cwd, session_types, keyword)?, + }; + let bytes = + serde_json::to_vec(&token).internal_err_ctx("Failed to encode session list cursor")?; + Ok(URL_SAFE_NO_PAD.encode(bytes)) +} + +impl GooseAcpAgent { + pub(super) async fn on_list_sessions( + &self, + req: ListSessionsRequest, + ) -> Result { + if let Some(cwd) = req.cwd.as_deref() { + if !cwd.is_absolute() { + return Err(agent_client_protocol::Error::invalid_params() + .data("cwd must be an absolute path")); + } + } + + let cwd = req.cwd.as_deref(); + let keyword = session_keyword_from_meta(req.meta.as_ref())?; + let session_types = session_types_from_meta(req.meta.as_ref())?; + let cursor = decode_session_list_cursor( + req.cursor.as_deref(), + cwd, + &session_types, + keyword.as_deref(), + )?; + + // ACP clients see their own (Acp) sessions plus legacy User/Scheduled ones. + let page = self + .session_manager + .list_sessions_paged(SessionListPageQuery { + filters: SessionListFilters { + types: Some(&session_types), + working_dir: cwd, + keyword: keyword.as_deref(), + only_sessions_with_messages: true, + }, + cursor: cursor.as_ref(), + page_size: SESSION_LIST_PAGE_SIZE, + }) + .await + .internal_err()?; + let session_infos: Vec = page + .sessions + .into_iter() + .map(|s| { + let meta = session_meta(&s); + let title = s.display_title(); + let mut info = SessionInfo::new(SessionId::new(s.id), s.working_dir) + .updated_at(s.updated_at.to_rfc3339()) + .meta(meta); + if let Some(t) = title { + info = info.title(t); + } + info + }) + .collect(); + let next_cursor = page + .next_cursor + .as_ref() + .map(|cursor| { + encode_session_list_cursor(cursor, cwd, &session_types, keyword.as_deref()) + }) + .transpose()?; + Ok(ListSessionsResponse::new(session_infos).next_cursor(next_cursor)) + } +} diff --git a/crates/goose/src/acp/server/new_session.rs b/crates/goose/src/acp/server/new_session.rs index c1a6d5c5354e..a17d568de30c 100644 --- a/crates/goose/src/acp/server/new_session.rs +++ b/crates/goose/src/acp/server/new_session.rs @@ -18,14 +18,14 @@ impl GooseAcpAgent { debug!(?args, "new session request"); let t_start = std::time::Instant::now(); validate_absolute_cwd(&args.cwd)?; - let project_id = meta_string(args.meta.as_ref(), "projectId"); - let session_type = match meta_string(args.meta.as_ref(), "client") { + let project_id = meta_string(args.meta.as_ref(), "projectId")?; + let session_type = match meta_string(args.meta.as_ref(), "client")? { Some(_) => SessionType::User, None => SessionType::Acp, }; let config = Config::global(); let (resolved_provider, resolved_model_config) = - match meta_string(args.meta.as_ref(), "provider") { + match meta_string(args.meta.as_ref(), "provider")? { Some(provider) => { let model_config = super::resolve_provider_default_model_config(&provider).await?; diff --git a/crates/goose/src/session/session_manager.rs b/crates/goose/src/session/session_manager.rs index f7205654ca6a..6a10a1abe92d 100644 --- a/crates/goose/src/session/session_manager.rs +++ b/crates/goose/src/session/session_manager.rs @@ -301,13 +301,57 @@ pub(crate) struct SessionListPage { pub(crate) next_cursor: Option, } +#[derive(Debug, Default, Clone)] +pub(crate) struct SessionListFilters<'a> { + pub(crate) types: Option<&'a [SessionType]>, + pub(crate) working_dir: Option<&'a Path>, + pub(crate) keyword: Option<&'a str>, + pub(crate) only_sessions_with_messages: bool, +} + +#[derive(Debug, Clone)] +pub(crate) struct SessionListPageQuery<'a> { + pub(crate) filters: SessionListFilters<'a>, + pub(crate) cursor: Option<&'a SessionListCursor>, + pub(crate) page_size: usize, +} + #[derive(Debug, Default)] struct SessionListQuery<'a> { - types: Option<&'a [SessionType]>, - working_dir: Option<&'a Path>, + filters: SessionListFilters<'a>, cursor: Option<&'a SessionListCursor>, limit: Option, - require_messages: bool, +} + +fn keyword_terms(query: Option<&str>) -> Vec { + query + .unwrap_or_default() + .split_whitespace() + .map(|word| word.to_lowercase()) + .collect() +} + +fn message_keyword_clause(keyword_count: usize) -> String { + let keyword_clauses = (0..keyword_count) + .map(|_| "instr(LOWER(json_extract(value, '$.text')), ?) > 0") + .collect::>() + .join(" OR "); + + format!( + r#" + EXISTS ( + SELECT 1 + FROM messages mq + WHERE mq.session_id = s.id + AND EXISTS ( + SELECT 1 + FROM json_each(mq.content_json) + WHERE json_extract(value, '$.type') = 'text' + AND ({keyword_clauses}) + ) + ) + "# + ) } #[derive(Debug, Clone)] @@ -376,16 +420,11 @@ impl SessionManager { self.storage.list_sessions_by_types(Some(types)).await } - pub(crate) async fn list_nonempty_sessions_by_types_paged( + pub(crate) async fn list_sessions_paged( &self, - types: &[SessionType], - working_dir: Option<&Path>, - cursor: Option<&SessionListCursor>, - page_size: usize, + query: SessionListPageQuery<'_>, ) -> Result { - self.storage - .list_nonempty_sessions_by_types_paged(types, working_dir, cursor, page_size) - .await + self.storage.list_sessions_paged(query).await } pub async fn list_all_sessions(&self) -> Result> { @@ -1547,20 +1586,25 @@ impl SessionStorage { Self::replace_conversation_inner(pool, session_id, conversation).await } - async fn list_sessions_matching(&self, options: SessionListQuery<'_>) -> Result> { - if matches!(options.types, Some(types) if types.is_empty()) { + async fn list_sessions_matching(&self, query: SessionListQuery<'_>) -> Result> { + let filters = &query.filters; + if matches!(filters.types, Some(types) if types.is_empty()) { return Ok(Vec::new()); } + let keywords = keyword_terms(filters.keyword); let mut where_clauses = Vec::new(); - if let Some(types) = options.types { + if let Some(types) = filters.types { let placeholders = types.iter().map(|_| "?").collect::>().join(", "); where_clauses.push(format!("s.session_type IN ({})", placeholders)); } - if options.working_dir.is_some() { + if filters.working_dir.is_some() { where_clauses.push("s.working_dir = ?".to_string()); } - if options.cursor.is_some() { + if !keywords.is_empty() { + where_clauses.push(message_keyword_clause(keywords.len())); + } + if query.cursor.is_some() { where_clauses.push( "(datetime(s.updated_at) < datetime(?) \ OR (datetime(s.updated_at) = datetime(?) AND s.id < ?))" @@ -1573,23 +1617,19 @@ impl SessionStorage { } else { format!("WHERE {}", where_clauses.join(" AND ")) }; - let message_join = if options.require_messages { + let message_join = if filters.only_sessions_with_messages { "JOIN messages m ON s.id = m.session_id" } else { "LEFT JOIN messages m ON s.id = m.session_id" }; - let order_by = if options.cursor.is_some() || options.limit.is_some() { + let order_by = if query.cursor.is_some() || query.limit.is_some() { "ORDER BY datetime(s.updated_at) DESC, s.id DESC" } else { "ORDER BY s.updated_at DESC" }; - let limit_clause = if options.limit.is_some() { - "LIMIT ?" - } else { - "" - }; + let limit_clause = if query.limit.is_some() { "LIMIT ?" } else { "" }; - let query = format!( + let sql = format!( r#" SELECT s.id, s.working_dir, s.name, s.description, s.user_set_name, s.session_type, s.created_at, s.updated_at, s.extension_data, s.total_tokens, s.input_tokens, s.output_tokens, @@ -1609,23 +1649,26 @@ impl SessionStorage { message_join, where_clause, order_by, limit_clause ); - let mut q = sqlx::query_as::<_, Session>(&query); - if let Some(types) = options.types { + let mut q = sqlx::query_as::<_, Session>(&sql); + if let Some(types) = filters.types { for session_type in types { q = q.bind(session_type.to_string()); } } - if let Some(working_dir) = options.working_dir { + if let Some(working_dir) = filters.working_dir { q = q.bind(working_dir.to_string_lossy().to_string()); } - if let Some(cursor) = options.cursor { + for term in keywords { + q = q.bind(term); + } + if let Some(cursor) = query.cursor { let updated_at = cursor.updated_at.to_rfc3339(); // Normalize mixed SQLite CURRENT_TIMESTAMP and RFC3339 stored values. q = q.bind(updated_at.clone()); q = q.bind(updated_at); q = q.bind(&cursor.session_id); } - if let Some(limit) = options.limit { + if let Some(limit) = query.limit { q = q.bind(limit as i64); } @@ -1635,33 +1678,32 @@ impl SessionStorage { async fn list_sessions_by_types(&self, types: Option<&[SessionType]>) -> Result> { self.list_sessions_matching(SessionListQuery { - types, + filters: SessionListFilters { + types, + ..Default::default() + }, ..Default::default() }) .await } - async fn list_nonempty_sessions_by_types_paged( + async fn list_sessions_paged( &self, - types: &[SessionType], - working_dir: Option<&Path>, - cursor: Option<&SessionListCursor>, - page_size: usize, + query: SessionListPageQuery<'_>, ) -> Result { - if types.is_empty() || page_size == 0 { + if matches!(query.filters.types, Some(types) if types.is_empty()) || query.page_size == 0 { return Ok(SessionListPage { sessions: Vec::new(), next_cursor: None, }); } + let page_size = query.page_size; let mut sessions = self .list_sessions_matching(SessionListQuery { - types: Some(types), - working_dir, - cursor, + filters: query.filters, + cursor: query.cursor, limit: Some(page_size + 1), - require_messages: true, }) .await?; let has_next_page = sessions.len() > page_size; @@ -2068,6 +2110,18 @@ mod tests { session.id } + async fn create_session_for_list_with_message( + sm: &SessionManager, + working_dir: &str, + message: &str, + ) -> String { + let session_id = create_session_for_list(sm, working_dir, false).await; + sm.add_message(&session_id, &Message::user().with_text(message)) + .await + .unwrap(); + session_id + } + async fn set_sessions_updated_at( sm: &SessionManager, session_ids: &[String], @@ -2321,13 +2375,18 @@ mod tests { expected_ids: &[String], expected_next_cursor: bool, ) -> Option { + let types = [SessionType::User]; let page = sm - .list_nonempty_sessions_by_types_paged( - &[SessionType::User], - working_dir.map(Path::new), + .list_sessions_paged(SessionListPageQuery { + filters: SessionListFilters { + types: Some(&types), + working_dir: working_dir.map(Path::new), + only_sessions_with_messages: true, + ..Default::default() + }, cursor, page_size, - ) + }) .await .unwrap(); let ids = page @@ -2496,6 +2555,229 @@ mod tests { .await; } + #[tokio::test] + async fn test_session_list_paged_filters_by_keyword() { + let temp_dir = TempDir::new().unwrap(); + let sm = SessionManager::new(temp_dir.path().to_path_buf()); + let target = create_session_for_list_with_message( + &sm, + "/tmp/session-list", + "Discuss Postgres migrations", + ) + .await; + create_session_for_list_with_message(&sm, "/tmp/session-list", "Plan the mobile release") + .await; + + let types = [SessionType::User]; + let page = sm + .list_sessions_paged(SessionListPageQuery { + filters: SessionListFilters { + types: Some(&types), + keyword: Some("postgres"), + only_sessions_with_messages: true, + ..Default::default() + }, + cursor: None, + page_size: 10, + }) + .await + .unwrap(); + let ids = page + .sessions + .iter() + .map(|session| session.id.clone()) + .collect::>(); + + assert_eq!(ids, vec![target]); + assert!(page.next_cursor.is_none()); + } + + #[tokio::test] + async fn test_session_list_paged_keyword_uses_or_terms() { + let temp_dir = TempDir::new().unwrap(); + let sm = SessionManager::new(temp_dir.path().to_path_buf()); + let postgres = create_session_for_list_with_message( + &sm, + "/tmp/session-list", + "Postgres migration plan", + ) + .await; + let sqlite = + create_session_for_list_with_message(&sm, "/tmp/session-list", "SQLite backup notes") + .await; + create_session_for_list_with_message(&sm, "/tmp/session-list", "Mobile release notes") + .await; + let expected_ids = expected_session_list_ids(&sm, &[postgres, sqlite]).await; + + let types = [SessionType::User]; + let page = sm + .list_sessions_paged(SessionListPageQuery { + filters: SessionListFilters { + types: Some(&types), + keyword: Some("postgres sqlite"), + only_sessions_with_messages: true, + ..Default::default() + }, + cursor: None, + page_size: 10, + }) + .await + .unwrap(); + let ids = page + .sessions + .iter() + .map(|session| session.id.clone()) + .collect::>(); + + assert_eq!(ids, expected_ids); + assert!(page.next_cursor.is_none()); + } + + #[tokio::test] + async fn test_session_list_paged_empty_keyword_matches_plain_list() { + let temp_dir = TempDir::new().unwrap(); + let sm = SessionManager::new(temp_dir.path().to_path_buf()); + let expected_ids = vec![ + create_session_for_list_with_message(&sm, "/tmp/session-list", "first message").await, + create_session_for_list_with_message(&sm, "/tmp/session-list", "second message").await, + ]; + let expected_ids = expected_session_list_ids(&sm, &expected_ids).await; + + let types = [SessionType::User]; + let page = sm + .list_sessions_paged(SessionListPageQuery { + filters: SessionListFilters { + types: Some(&types), + keyword: Some(" "), + only_sessions_with_messages: true, + ..Default::default() + }, + cursor: None, + page_size: 10, + }) + .await + .unwrap(); + let ids = page + .sessions + .iter() + .map(|session| session.id.clone()) + .collect::>(); + + assert_eq!(ids, expected_ids); + } + + #[tokio::test] + async fn test_session_list_paged_keyword_treats_like_wildcards_as_literals() { + let temp_dir = TempDir::new().unwrap(); + let sm = SessionManager::new(temp_dir.path().to_path_buf()); + let percent_id = + create_session_for_list_with_message(&sm, "/tmp/session-list", "Deploy is 100% done") + .await; + let underscore_id = create_session_for_list_with_message( + &sm, + "/tmp/session-list", + "feature_flag is enabled", + ) + .await; + create_session_for_list_with_message(&sm, "/tmp/session-list", "plain message").await; + + let types = [SessionType::User]; + let percent_page = sm + .list_sessions_paged(SessionListPageQuery { + filters: SessionListFilters { + types: Some(&types), + keyword: Some("%"), + only_sessions_with_messages: true, + ..Default::default() + }, + cursor: None, + page_size: 10, + }) + .await + .unwrap(); + let percent_ids = percent_page + .sessions + .iter() + .map(|session| session.id.clone()) + .collect::>(); + assert_eq!(percent_ids, vec![percent_id]); + + let underscore_page = sm + .list_sessions_paged(SessionListPageQuery { + filters: SessionListFilters { + types: Some(&types), + keyword: Some("_"), + only_sessions_with_messages: true, + ..Default::default() + }, + cursor: None, + page_size: 10, + }) + .await + .unwrap(); + let underscore_ids = underscore_page + .sessions + .iter() + .map(|session| session.id.clone()) + .collect::>(); + assert_eq!(underscore_ids, vec![underscore_id]); + } + + #[tokio::test] + async fn test_session_list_paged_keyword_combines_with_cwd_and_pagination() { + let temp_dir = TempDir::new().unwrap(); + let sm = SessionManager::new(temp_dir.path().to_path_buf()); + let expected_ids = vec![ + create_session_for_list_with_message(&sm, "/tmp/session-list/a", "Postgres plan one") + .await, + create_session_for_list_with_message(&sm, "/tmp/session-list/a", "Postgres plan two") + .await, + ]; + create_session_for_list_with_message(&sm, "/tmp/session-list/a", "Mobile release").await; + create_session_for_list_with_message(&sm, "/tmp/session-list/b", "Postgres plan other") + .await; + let expected_ids = expected_session_list_ids(&sm, &expected_ids).await; + + let types = [SessionType::User]; + let filters = SessionListFilters { + types: Some(&types), + working_dir: Some(Path::new("/tmp/session-list/a")), + keyword: Some("postgres"), + only_sessions_with_messages: true, + }; + let cursor = sm + .list_sessions_paged(SessionListPageQuery { + filters: filters.clone(), + cursor: None, + page_size: 1, + }) + .await + .unwrap(); + let ids = cursor + .sessions + .iter() + .map(|session| session.id.clone()) + .collect::>(); + assert_eq!(ids, expected_ids[0..1]); + assert!(cursor.next_cursor.is_some()); + + let page = sm + .list_sessions_paged(SessionListPageQuery { + filters, + cursor: cursor.next_cursor.as_ref(), + page_size: 1, + }) + .await + .unwrap(); + let ids = page + .sessions + .iter() + .map(|session| session.id.clone()) + .collect::>(); + assert_eq!(ids, expected_ids[1..2]); + assert!(page.next_cursor.is_none()); + } + #[tokio::test] async fn test_concurrent_session_creation() { let temp_dir = TempDir::new().unwrap(); diff --git a/crates/goose/tests/acp_server_test.rs b/crates/goose/tests/acp_server_test.rs index 22cbfa32d76d..4fb7bbd5a5a1 100644 --- a/crates/goose/tests/acp_server_test.rs +++ b/crates/goose/tests/acp_server_test.rs @@ -46,6 +46,29 @@ async fn seed_list_sessions(data_root: &Path, working_dir: &Path, count: usize) } } +async fn seed_list_session_with_message( + data_root: &Path, + working_dir: &Path, + name: &str, + session_type: SessionType, + message: &str, +) { + let session_manager = SessionManager::new(data_root.to_path_buf()); + let session = session_manager + .create_session( + working_dir.to_path_buf(), + name.to_string(), + session_type, + GooseMode::default(), + ) + .await + .unwrap(); + session_manager + .add_message(&session.id, &Message::user().with_text(message)) + .await + .unwrap(); +} + async fn new_connection(data_root: &Path) -> AcpServerConnection { let openai = OpenAiFixture::new( vec![], @@ -122,6 +145,106 @@ fn test_list_sessions_pagination() { }); } +#[test] +fn test_list_sessions_query_filters_results() { + run_test(async { + let data_root = tempfile::tempdir().unwrap(); + let cwd = Path::new("/tmp/acp-session-list"); + seed_list_session_with_message( + data_root.path(), + cwd, + "Postgres session", + SessionType::Acp, + "Discuss Postgres migrations", + ) + .await; + seed_list_session_with_message( + data_root.path(), + cwd, + "Mobile session", + SessionType::Acp, + "Plan the mobile release", + ) + .await; + let conn = new_connection(data_root.path()).await; + + let mut meta = serde_json::Map::new(); + meta.insert( + "query".to_string(), + serde_json::Value::String("postgres".to_string()), + ); + let response = list_sessions_request(&conn, ListSessionsRequest::new().meta(meta)) + .await + .unwrap(); + + assert_eq!(response.sessions.len(), 1); + assert_eq!( + response.sessions[0].title.as_deref(), + Some("Postgres session") + ); + assert!(response.next_cursor.is_none()); + }); +} + +#[test] +fn test_list_sessions_types_override_filters_results() { + run_test(async { + let data_root = tempfile::tempdir().unwrap(); + let cwd = Path::new("/tmp/acp-session-list"); + seed_list_session_with_message( + data_root.path(), + cwd, + "ACP session", + SessionType::Acp, + "ACP message", + ) + .await; + seed_list_session_with_message( + data_root.path(), + cwd, + "User session", + SessionType::User, + "User message", + ) + .await; + let conn = new_connection(data_root.path()).await; + + let mut meta = serde_json::Map::new(); + meta.insert( + "types".to_string(), + serde_json::Value::Array(vec![serde_json::Value::String("user".to_string())]), + ); + let response = list_sessions_request(&conn, ListSessionsRequest::new().meta(meta)) + .await + .unwrap(); + + assert_eq!(response.sessions.len(), 1); + assert_eq!(response.sessions[0].title.as_deref(), Some("User session")); + assert!(response.next_cursor.is_none()); + }); +} + +#[test] +fn test_list_sessions_types_rejects_internal_session_types() { + run_test(async { + let data_root = tempfile::tempdir().unwrap(); + let conn = new_connection(data_root.path()).await; + + for session_type in ["hidden", "sub_agent"] { + let mut meta = serde_json::Map::new(); + meta.insert( + "types".to_string(), + serde_json::Value::Array(vec![serde_json::Value::String(session_type.to_string())]), + ); + + let error = list_sessions_request(&conn, ListSessionsRequest::new().meta(meta)) + .await + .unwrap_err(); + assert_invalid_params(error); + } + }); +} + #[test] fn test_list_sessions_invalid_params() { run_test(async {