mirror of
https://github.com/openai/codex.git
synced 2026-11-15 21:50:46 +00:00
Allow searching persisted attached threads (#60574)
## Why Title and content searches exclude attached threads, preventing clients from finding their saved conversations alongside ordinary threads. ## What changed - Add `includeAttachedThreads` to `thread/list` and `thread/search`. For `thread/list`, require a non-empty `searchTerm` and preserve the usual provider and source filters. - Expose `attachedToThreadId` on thread responses to identify the owning parent independently of fork and subagent lineage. - Support inclusive searches through SQLite and filesystem fallback, repairing missing database entries with saved thread names during title searches. ## Testing Add regression coverage for opt-in title and content searches, attached-parent metadata after restart, default search filters, filesystem searches without SQLite, and discovery and repair of unindexed title matches. GitOrigin-RevId: ac12ec3c730ad7f1722bdf8e21600d17f12017f0
This commit is contained in:
@@ -1007,10 +1007,9 @@ impl ThreadStore for InMemoryThreadStore {
|
||||
page.items
|
||||
.retain(|thread| thread.parent_thread_id == Some(parent_thread_id));
|
||||
}
|
||||
Some(ThreadRelationFilter::AttachedThreadsOf(_)) => {
|
||||
return Err(ThreadStoreError::Unsupported {
|
||||
operation: "attached threads",
|
||||
});
|
||||
Some(ThreadRelationFilter::AttachedThreadsOf(parent)) => {
|
||||
page.items
|
||||
.retain(|thread| thread.attached_to_thread_id == Some(parent));
|
||||
}
|
||||
Some(ThreadRelationFilter::DescendantsOf(ancestor_thread_id)) => {
|
||||
let mut subtree = HashSet::from([ancestor_thread_id]);
|
||||
@@ -1033,7 +1032,10 @@ impl ThreadStore for InMemoryThreadStore {
|
||||
&& subtree.contains(&thread.thread_id)
|
||||
});
|
||||
}
|
||||
None => {}
|
||||
Some(ThreadRelationFilter::IncludeAttached) => {}
|
||||
None => page
|
||||
.items
|
||||
.retain(|thread| thread.attached_to_thread_id.is_none()),
|
||||
}
|
||||
if let Some(section) = params.section.as_ref() {
|
||||
page.items.retain(|thread| {
|
||||
@@ -1132,6 +1134,7 @@ fn stored_thread_from_state(
|
||||
.and_then(|metadata| metadata.rollout_path.clone())
|
||||
.or(rollout_path),
|
||||
forked_from_id: created.forked_from_id,
|
||||
attached_to_thread_id: created.forked_from_id.filter(|_| created.attached_thread),
|
||||
parent_thread_id: created.parent_thread_id,
|
||||
preview: metadata
|
||||
.and_then(|metadata| metadata.preview.clone())
|
||||
|
||||
@@ -146,6 +146,7 @@ pub(super) fn stored_thread_from_rollout_item(
|
||||
extra_config: None,
|
||||
rollout_path: Some(rollout_path),
|
||||
forked_from_id: None,
|
||||
attached_to_thread_id: item.attached_to_thread_id,
|
||||
parent_thread_id: item.parent_thread_id,
|
||||
preview,
|
||||
name: codex_state::is_guardian_review_source(&source)
|
||||
|
||||
@@ -241,7 +241,13 @@ pub(super) async fn list_rollout_threads(
|
||||
sort_key: codex_rollout::ThreadSortKey,
|
||||
sort_direction: codex_rollout::SortDirection,
|
||||
) -> ThreadStoreResult<codex_rollout::ThreadsPage> {
|
||||
if params.relation_filter.is_some() || params.section.is_some() || params.project_id.is_some() {
|
||||
if params.use_state_db_only
|
||||
|| params
|
||||
.relation_filter
|
||||
.is_some_and(ThreadRelationFilter::is_parent_scoped)
|
||||
|| params.section.is_some()
|
||||
|| params.project_id.is_some()
|
||||
{
|
||||
let page = codex_rollout::state_db::list_threads_db(
|
||||
state_db.as_deref(),
|
||||
&config.sqlite,
|
||||
@@ -265,37 +271,9 @@ pub(super) async fn list_rollout_threads(
|
||||
return Ok(page.into());
|
||||
}
|
||||
|
||||
let page = if params.use_state_db_only && params.archived {
|
||||
RolloutRecorder::list_archived_threads_from_state_db(
|
||||
state_db,
|
||||
config,
|
||||
params.page_size,
|
||||
cursor,
|
||||
sort_key,
|
||||
sort_direction,
|
||||
params.allowed_sources.as_slice(),
|
||||
params.model_providers.as_deref(),
|
||||
params.cwd_filters.as_deref(),
|
||||
default_model_provider_id,
|
||||
params.search_term.as_deref(),
|
||||
)
|
||||
.await
|
||||
} else if params.use_state_db_only {
|
||||
RolloutRecorder::list_threads_from_state_db(
|
||||
state_db,
|
||||
config,
|
||||
params.page_size,
|
||||
cursor,
|
||||
sort_key,
|
||||
sort_direction,
|
||||
params.allowed_sources.as_slice(),
|
||||
params.model_providers.as_deref(),
|
||||
params.cwd_filters.as_deref(),
|
||||
default_model_provider_id,
|
||||
params.search_term.as_deref(),
|
||||
)
|
||||
.await
|
||||
} else if params.archived {
|
||||
let include_attached_threads =
|
||||
params.relation_filter == Some(ThreadRelationFilter::IncludeAttached);
|
||||
let page = if params.archived {
|
||||
RolloutRecorder::list_archived_threads(
|
||||
state_db,
|
||||
config,
|
||||
@@ -308,6 +286,7 @@ pub(super) async fn list_rollout_threads(
|
||||
params.cwd_filters.as_deref(),
|
||||
default_model_provider_id,
|
||||
params.search_term.as_deref(),
|
||||
include_attached_threads,
|
||||
)
|
||||
.await
|
||||
} else {
|
||||
@@ -323,6 +302,7 @@ pub(super) async fn list_rollout_threads(
|
||||
params.cwd_filters.as_deref(),
|
||||
default_model_provider_id,
|
||||
params.search_term.as_deref(),
|
||||
include_attached_threads,
|
||||
)
|
||||
.await
|
||||
};
|
||||
@@ -333,6 +313,7 @@ pub(super) async fn list_rollout_threads(
|
||||
|
||||
fn state_relation_filter(filter: ThreadRelationFilter) -> codex_state::ThreadRelationFilter {
|
||||
match filter {
|
||||
ThreadRelationFilter::IncludeAttached => codex_state::ThreadRelationFilter::IncludeAttached,
|
||||
ThreadRelationFilter::DirectChildrenOf(id) => {
|
||||
codex_state::ThreadRelationFilter::DirectChildrenOf(id)
|
||||
}
|
||||
|
||||
@@ -455,6 +455,7 @@ pub(super) fn stored_thread_from_state_metadata(
|
||||
thread_id: metadata.id,
|
||||
extra_config: None,
|
||||
rollout_path: Some(rollout_path),
|
||||
attached_to_thread_id: metadata.attached_thread_parent_id,
|
||||
forked_from_id: None,
|
||||
parent_thread_id,
|
||||
preview,
|
||||
@@ -560,6 +561,10 @@ fn stored_thread_from_meta_line(
|
||||
thread_id: meta_line.meta.id,
|
||||
extra_config: None,
|
||||
rollout_path: Some(rollout_path),
|
||||
attached_to_thread_id: meta_line
|
||||
.meta
|
||||
.forked_from_id
|
||||
.filter(|_| meta_line.meta.attached_thread),
|
||||
forked_from_id: meta_line.meta.forked_from_id,
|
||||
parent_thread_id: meta_line.meta.parent_thread_id,
|
||||
preview: String::new(),
|
||||
@@ -864,6 +869,7 @@ mod tests {
|
||||
"Forked user message",
|
||||
Some("test-provider"),
|
||||
Some(parent_uuid),
|
||||
/*attached_thread*/ false,
|
||||
ThreadHistoryMode::Legacy,
|
||||
)
|
||||
.expect("forked session file");
|
||||
|
||||
@@ -106,7 +106,9 @@ pub(super) async fn search_threads(
|
||||
project_id: None,
|
||||
archived: params.archived,
|
||||
search_term: None,
|
||||
relation_filter: None,
|
||||
relation_filter: params
|
||||
.include_attached_threads
|
||||
.then_some(crate::ThreadRelationFilter::IncludeAttached),
|
||||
use_state_db_only: state_db.is_some(),
|
||||
};
|
||||
let mut remaining_rollouts = matching_rollouts
|
||||
|
||||
@@ -1,3 +1,7 @@
|
||||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use chrono::Utc;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
@@ -10,10 +14,13 @@ use uuid::Uuid;
|
||||
use super::super::LocalThreadStore;
|
||||
use super::super::test_support::test_config;
|
||||
use super::super::test_support::write_session_file_with;
|
||||
use super::super::test_support::write_session_file_with_fork;
|
||||
use super::ThreadSearchItem;
|
||||
use super::cursor_from_thread_search_item;
|
||||
use crate::ListThreadsParams;
|
||||
use crate::SearchThreadsParams;
|
||||
use crate::SortDirection;
|
||||
use crate::ThreadRelationFilter;
|
||||
use crate::ThreadSortKey;
|
||||
use crate::ThreadStore;
|
||||
|
||||
@@ -141,6 +148,7 @@ async fn search_matches_selected_rollout_across_path_spellings_and_compression()
|
||||
] {
|
||||
let page = store
|
||||
.search_threads(SearchThreadsParams {
|
||||
include_attached_threads: false,
|
||||
account: None,
|
||||
page_size: 10,
|
||||
cursor: None,
|
||||
@@ -168,3 +176,147 @@ async fn search_matches_selected_rollout_across_path_spellings_and_compression()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn write_named_rollout(home: &Path, id: Uuid, attached_to: Option<Uuid>) -> PathBuf {
|
||||
let path = write_session_file_with_fork(
|
||||
home,
|
||||
home.join("sessions/2025/01/03"),
|
||||
"2025-01-03T12-00-00",
|
||||
id,
|
||||
"searchable message",
|
||||
Some("test-provider"),
|
||||
attached_to,
|
||||
attached_to.is_some(),
|
||||
ThreadHistoryMode::Legacy,
|
||||
)
|
||||
.expect("write rollout");
|
||||
codex_rollout::append_thread_name(home, ThreadId::from_u128(id.as_u128()), "matching title")
|
||||
.await
|
||||
.expect("save thread name");
|
||||
path
|
||||
}
|
||||
|
||||
fn inclusive_title_search() -> ListThreadsParams {
|
||||
ListThreadsParams {
|
||||
page_size: 10,
|
||||
cursor: None,
|
||||
sort_key: ThreadSortKey::CreatedAt,
|
||||
sort_direction: SortDirection::Desc,
|
||||
allowed_sources: vec![SessionSource::Cli],
|
||||
model_providers: None,
|
||||
cwd_filters: None,
|
||||
section: None,
|
||||
project_id: None,
|
||||
archived: false,
|
||||
search_term: Some("matching title".to_string()),
|
||||
relation_filter: Some(ThreadRelationFilter::IncludeAttached),
|
||||
use_state_db_only: false,
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn title_and_content_search_return_attached_parent_without_state_db() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let store = LocalThreadStore::new(test_config(home.path()), /*state_db*/ None);
|
||||
let parent = Uuid::new_v4();
|
||||
let child = Uuid::new_v4();
|
||||
write_named_rollout(home.path(), child, Some(parent)).await;
|
||||
let expected = vec![(
|
||||
ThreadId::from_u128(child.as_u128()),
|
||||
Some(ThreadId::from_u128(parent.as_u128())),
|
||||
)];
|
||||
|
||||
let list = store
|
||||
.list_threads(inclusive_title_search())
|
||||
.await
|
||||
.expect("search titles on disk");
|
||||
assert_eq!(
|
||||
list.items
|
||||
.into_iter()
|
||||
.map(|t| (t.thread_id, t.attached_to_thread_id))
|
||||
.collect::<Vec<_>>(),
|
||||
expected
|
||||
);
|
||||
|
||||
let search = store
|
||||
.search_threads(SearchThreadsParams {
|
||||
page_size: 10,
|
||||
cursor: None,
|
||||
sort_key: ThreadSortKey::CreatedAt,
|
||||
sort_direction: SortDirection::Desc,
|
||||
allowed_sources: vec![SessionSource::Cli],
|
||||
archived: false,
|
||||
search_term: "searchable message".to_string(),
|
||||
include_attached_threads: true,
|
||||
account: None,
|
||||
})
|
||||
.await
|
||||
.expect("search messages on disk");
|
||||
assert_eq!(
|
||||
search
|
||||
.items
|
||||
.into_iter()
|
||||
.map(|hit| (hit.thread.thread_id, hit.thread.attached_to_thread_id))
|
||||
.collect::<Vec<_>>(),
|
||||
expected
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn inclusive_title_search_finds_unindexed_matches_when_filesystem_scanning_is_allowed() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let config = test_config(home.path());
|
||||
let state_db = codex_state::StateRuntime::init(
|
||||
config.sqlite.clone(),
|
||||
config.default_model_provider_id.clone(),
|
||||
)
|
||||
.await
|
||||
.expect("initialize state database");
|
||||
let parent = Uuid::new_v4();
|
||||
let child = Uuid::new_v4();
|
||||
let standalone = Uuid::new_v4();
|
||||
let parent_path = write_named_rollout(home.path(), parent, /*attached_to*/ None).await;
|
||||
write_named_rollout(home.path(), standalone, /*attached_to*/ None).await;
|
||||
write_named_rollout(home.path(), child, Some(parent)).await;
|
||||
let parent_id = ThreadId::from_u128(parent.as_u128());
|
||||
let child_id = ThreadId::from_u128(child.as_u128());
|
||||
let mut row = codex_state::ThreadMetadataBuilder::new(
|
||||
parent_id,
|
||||
parent_path,
|
||||
Utc::now(),
|
||||
SessionSource::Cli,
|
||||
)
|
||||
.build("test-provider");
|
||||
row.first_user_message = Some("searchable message".to_string());
|
||||
row.title = "matching title".to_string();
|
||||
state_db
|
||||
.upsert_thread(&row)
|
||||
.await
|
||||
.expect("index only the parent");
|
||||
let store = LocalThreadStore::new(config, Some(state_db));
|
||||
let all = HashMap::from([
|
||||
(parent_id, None),
|
||||
(child_id, Some(parent_id)),
|
||||
(ThreadId::from_u128(standalone.as_u128()), None),
|
||||
]);
|
||||
for (use_state_db_only, expected) in [
|
||||
(true, HashMap::from([(parent_id, None)])),
|
||||
(false, all.clone()),
|
||||
(true, all),
|
||||
] {
|
||||
let page = store
|
||||
.list_threads(ListThreadsParams {
|
||||
use_state_db_only,
|
||||
..inclusive_title_search()
|
||||
})
|
||||
.await
|
||||
.expect("search titles");
|
||||
assert_eq!(
|
||||
page.items
|
||||
.into_iter()
|
||||
.map(|t| (t.thread_id, t.attached_to_thread_id))
|
||||
.collect::<HashMap<_, _>>(),
|
||||
expected
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -72,6 +72,7 @@ pub(super) fn write_session_file_with(
|
||||
first_user_message,
|
||||
model_provider,
|
||||
/*forked_from_id*/ None,
|
||||
/*attached_thread*/ false,
|
||||
history_mode,
|
||||
)
|
||||
}
|
||||
@@ -85,6 +86,7 @@ pub(super) fn write_session_file_with_fork(
|
||||
first_user_message: &str,
|
||||
model_provider: Option<&str>,
|
||||
forked_from_id: Option<Uuid>,
|
||||
attached_thread: bool,
|
||||
history_mode: ThreadHistoryMode,
|
||||
) -> std::io::Result<PathBuf> {
|
||||
fs::create_dir_all(&day_dir)?;
|
||||
@@ -97,6 +99,7 @@ pub(super) fn write_session_file_with_fork(
|
||||
"session_id": uuid,
|
||||
"id": uuid,
|
||||
"forked_from_id": forked_from_id,
|
||||
"attached_thread": attached_thread,
|
||||
"timestamp": ts,
|
||||
"cwd": root,
|
||||
"originator": "test_originator",
|
||||
|
||||
@@ -337,6 +337,8 @@ pub enum SortDirection {
|
||||
/// Parent relationship used to filter thread listings.
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub enum ThreadRelationFilter {
|
||||
/// Include attached threads alongside ordinary threads for search.
|
||||
IncludeAttached,
|
||||
/// Return only threads whose immediate parent is the given thread.
|
||||
DirectChildrenOf(ThreadId),
|
||||
/// Return every thread transitively descended from the given thread.
|
||||
@@ -345,6 +347,16 @@ pub enum ThreadRelationFilter {
|
||||
AttachedThreadsOf(ThreadId),
|
||||
}
|
||||
|
||||
impl ThreadRelationFilter {
|
||||
/// Whether the caller requested relatives of one particular thread.
|
||||
pub fn is_parent_scoped(self) -> bool {
|
||||
match self {
|
||||
Self::IncludeAttached => false,
|
||||
Self::DirectChildrenOf(_) | Self::DescendantsOf(_) | Self::AttachedThreadsOf(_) => true,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Parameters for listing threads.
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct ListThreadsParams {
|
||||
@@ -383,6 +395,8 @@ pub struct ListThreadsParams {
|
||||
/// Parameters for searching thread content.
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct SearchThreadsParams {
|
||||
#[serde(default)]
|
||||
pub include_attached_threads: bool,
|
||||
/// The current login when searching account-scoped local history.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub account: Option<crate::LocalHistoryAccount>,
|
||||
@@ -660,6 +674,9 @@ pub struct StoredThread {
|
||||
pub rollout_path: Option<PathBuf>,
|
||||
/// Source thread id when this thread was forked from another thread.
|
||||
pub forked_from_id: Option<ThreadId>,
|
||||
/// Parent that owns this persisted attached thread, independent of fork or spawn lineage.
|
||||
#[serde(default)]
|
||||
pub attached_to_thread_id: Option<ThreadId>,
|
||||
/// The ID of the parent thread. This will only be set if this thread is a subagent.
|
||||
pub parent_thread_id: Option<ThreadId>,
|
||||
/// Best available user-facing preview, usually the first user message.
|
||||
|
||||
Reference in New Issue
Block a user