mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat(app-server): filter threads by parent (#26662)
## Why Clients that display or coordinate spawned subagents need an authoritative snapshot of a thread's immediate spawned children when they connect to app-server or recover after missing live events. `thread/list` cannot query by parent, so clients must otherwise scan unrelated threads or reconstruct relationships from rollout history and transient events. The direct spawn relationship already exists in persisted `thread_spawn_edges` state. Review and Guardian threads do not participate in that lifecycle and are intentionally outside this filter's scope. ## What changed This adds an experimental `parentThreadId` filter to `thread/list`. Parent-filtered requests return direct spawned children from persisted state while preserving the existing response shape, explicit filters, sorting, and timestamp-only cursor behavior. The lookup does not read rollout transcripts or recursively return descendants. Supersedes #25112 with the narrower `thread/list` filter approach. ## How it works 1. An experimental client passes a valid thread ID as `parentThreadId`. 2. App-server routes the list through the existing thread-store and state-database boundaries. 3. SQLite selects threads whose IDs have a direct persisted spawn edge from that parent. 4. Omitted provider and source filters include all values; explicit filters keep ordinary `thread/list` semantics. 5. Grandchildren, Review threads, and Guardian threads are excluded. ## Verification State (144 tests), rollout (69 tests), and focused app-server thread-list (31 tests) suites passed. Scoped Clippy checks and repository formatting also passed. Coverage includes direct spawned children, omitted grandchildren, pagination, malformed IDs, mixed source kinds, explicit filters, and operation without rollout files.
This commit is contained in:
committed by
GitHub
Unverified
parent
efbd00f21f
commit
dfd03ea01b
@@ -48,6 +48,10 @@ mod tests {
|
||||
use crate::ListTurnsParams;
|
||||
use crate::SortDirection;
|
||||
use crate::StoredTurnItemsView;
|
||||
use crate::ThreadPersistenceMetadata;
|
||||
use crate::ThreadSortKey;
|
||||
use codex_protocol::models::BaseInstructions;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
|
||||
#[tokio::test]
|
||||
async fn default_turn_pagination_methods_return_unsupported() {
|
||||
@@ -90,6 +94,68 @@ mod tests {
|
||||
}
|
||||
));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn list_threads_filters_by_parent_thread_id() {
|
||||
let store = InMemoryThreadStore::default();
|
||||
let parent_thread_id = ThreadId::default();
|
||||
let child_thread_id =
|
||||
ThreadId::from_string("00000000-0000-0000-0000-000000000001").expect("valid thread id");
|
||||
let unrelated_thread_id =
|
||||
ThreadId::from_string("00000000-0000-0000-0000-000000000002").expect("valid thread id");
|
||||
|
||||
for (thread_id, parent_thread_id) in [
|
||||
(child_thread_id, Some(parent_thread_id)),
|
||||
(unrelated_thread_id, None),
|
||||
] {
|
||||
store
|
||||
.create_thread(CreateThreadParams {
|
||||
thread_id,
|
||||
extra_config: None,
|
||||
forked_from_id: None,
|
||||
parent_thread_id,
|
||||
source: SessionSource::Exec,
|
||||
thread_source: None,
|
||||
base_instructions: BaseInstructions::default(),
|
||||
dynamic_tools: Vec::new(),
|
||||
multi_agent_version: None,
|
||||
metadata: ThreadPersistenceMetadata {
|
||||
cwd: None,
|
||||
model_provider: "test-provider".to_string(),
|
||||
memory_mode: ThreadMemoryMode::Enabled,
|
||||
},
|
||||
})
|
||||
.await
|
||||
.expect("create thread");
|
||||
}
|
||||
|
||||
let page = ThreadStore::list_threads(
|
||||
&store,
|
||||
ListThreadsParams {
|
||||
page_size: 10,
|
||||
cursor: None,
|
||||
sort_key: ThreadSortKey::CreatedAt,
|
||||
sort_direction: SortDirection::Desc,
|
||||
allowed_sources: Vec::new(),
|
||||
model_providers: None,
|
||||
cwd_filters: None,
|
||||
archived: false,
|
||||
search_term: None,
|
||||
parent_thread_id: Some(parent_thread_id),
|
||||
use_state_db_only: false,
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("list child threads");
|
||||
|
||||
assert_eq!(
|
||||
page.items
|
||||
.into_iter()
|
||||
.map(|item| item.thread_id)
|
||||
.collect::<Vec<_>>(),
|
||||
vec![child_thread_id]
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn stores_guard() -> MutexGuard<'static, HashMap<String, Arc<InMemoryThreadStore>>> {
|
||||
@@ -385,8 +451,15 @@ impl ThreadStore for InMemoryThreadStore {
|
||||
))
|
||||
}
|
||||
|
||||
fn list_threads(&self, _params: ListThreadsParams) -> ThreadStoreFuture<'_, ThreadPage> {
|
||||
Box::pin(InMemoryThreadStore::list_threads(self))
|
||||
fn list_threads(&self, params: ListThreadsParams) -> ThreadStoreFuture<'_, ThreadPage> {
|
||||
Box::pin(async move {
|
||||
let mut page = InMemoryThreadStore::list_threads(self).await?;
|
||||
if let Some(parent_thread_id) = params.parent_thread_id {
|
||||
page.items
|
||||
.retain(|thread| thread.parent_thread_id == Some(parent_thread_id));
|
||||
}
|
||||
Ok(page)
|
||||
})
|
||||
}
|
||||
|
||||
fn update_thread_metadata(
|
||||
|
||||
@@ -110,6 +110,7 @@ mod tests {
|
||||
cwd_filters: None,
|
||||
archived: true,
|
||||
search_term: None,
|
||||
parent_thread_id: None,
|
||||
use_state_db_only: false,
|
||||
})
|
||||
.await
|
||||
|
||||
@@ -25,7 +25,7 @@ pub(super) async fn create_thread(
|
||||
model_provider_id: params.metadata.model_provider.clone(),
|
||||
generate_memories: matches!(params.metadata.memory_mode, ThreadMemoryMode::Enabled),
|
||||
};
|
||||
let recorder = RolloutRecorder::new(
|
||||
RolloutRecorder::new(
|
||||
&config,
|
||||
RolloutRecorderParams::new(
|
||||
params.thread_id,
|
||||
@@ -41,7 +41,5 @@ pub(super) async fn create_thread(
|
||||
.await
|
||||
.map_err(|err| ThreadStoreError::Internal {
|
||||
message: format!("failed to initialize local thread recorder: {err}"),
|
||||
})?;
|
||||
|
||||
Ok(recorder)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -116,6 +116,32 @@ pub(super) async fn list_rollout_threads(
|
||||
sort_key: codex_rollout::ThreadSortKey,
|
||||
sort_direction: codex_rollout::SortDirection,
|
||||
) -> ThreadStoreResult<codex_rollout::ThreadsPage> {
|
||||
if let Some(parent_thread_id) = params.parent_thread_id {
|
||||
let page = codex_rollout::state_db::list_threads_db(
|
||||
state_db.as_deref(),
|
||||
config.codex_home.as_path(),
|
||||
params.page_size,
|
||||
cursor,
|
||||
sort_key,
|
||||
sort_direction,
|
||||
params.allowed_sources.as_slice(),
|
||||
params.model_providers.as_deref(),
|
||||
params.cwd_filters.as_deref(),
|
||||
Some(parent_thread_id),
|
||||
params.archived,
|
||||
params.search_term.as_deref(),
|
||||
)
|
||||
.await
|
||||
.ok_or_else(|| ThreadStoreError::Internal {
|
||||
message: "state DB unavailable for parent-filtered thread listing".to_string(),
|
||||
})?;
|
||||
let mut page: codex_rollout::ThreadsPage = page.into();
|
||||
for item in &mut page.items {
|
||||
item.parent_thread_id = Some(parent_thread_id);
|
||||
}
|
||||
return Ok(page);
|
||||
}
|
||||
|
||||
let page = if params.use_state_db_only && params.archived {
|
||||
RolloutRecorder::list_archived_threads_from_state_db(
|
||||
state_db,
|
||||
@@ -225,6 +251,7 @@ mod tests {
|
||||
cwd_filters: None,
|
||||
archived: false,
|
||||
search_term: None,
|
||||
parent_thread_id: None,
|
||||
use_state_db_only: false,
|
||||
})
|
||||
.await
|
||||
@@ -284,6 +311,7 @@ mod tests {
|
||||
cwd_filters: None,
|
||||
archived: false,
|
||||
search_term: Some("needle".to_string()),
|
||||
parent_thread_id: None,
|
||||
use_state_db_only: true,
|
||||
})
|
||||
.await
|
||||
@@ -323,6 +351,7 @@ mod tests {
|
||||
cwd_filters: None,
|
||||
archived: false,
|
||||
search_term: None,
|
||||
parent_thread_id: None,
|
||||
use_state_db_only: false,
|
||||
})
|
||||
.await
|
||||
@@ -338,6 +367,7 @@ mod tests {
|
||||
cwd_filters: None,
|
||||
archived: true,
|
||||
search_term: None,
|
||||
parent_thread_id: None,
|
||||
use_state_db_only: false,
|
||||
})
|
||||
.await
|
||||
@@ -389,6 +419,7 @@ mod tests {
|
||||
cwd_filters: None,
|
||||
archived: false,
|
||||
search_term: None,
|
||||
parent_thread_id: None,
|
||||
use_state_db_only: false,
|
||||
})
|
||||
.await
|
||||
@@ -425,6 +456,7 @@ mod tests {
|
||||
cwd_filters: None,
|
||||
archived: false,
|
||||
search_term: None,
|
||||
parent_thread_id: None,
|
||||
use_state_db_only: false,
|
||||
})
|
||||
.await
|
||||
|
||||
@@ -93,6 +93,7 @@ pub(super) async fn search_threads(
|
||||
cwd_filters: None,
|
||||
archived: params.archived,
|
||||
search_term: None,
|
||||
parent_thread_id: None,
|
||||
use_state_db_only: state_db.is_some(),
|
||||
};
|
||||
let mut remaining_rollouts = matching_rollouts;
|
||||
|
||||
@@ -1476,6 +1476,7 @@ mod tests {
|
||||
cwd_filters: Some(vec![workspace]),
|
||||
archived: false,
|
||||
search_term: None,
|
||||
parent_thread_id: None,
|
||||
use_state_db_only: true,
|
||||
})
|
||||
.await
|
||||
|
||||
@@ -195,6 +195,8 @@ pub struct ListThreadsParams {
|
||||
pub archived: bool,
|
||||
/// Optional substring/full-text search term for thread title/preview.
|
||||
pub search_term: Option<String>,
|
||||
/// Optional direct parent thread filter.
|
||||
pub parent_thread_id: Option<ThreadId>,
|
||||
/// Return directly from the state DB without scanning JSONL rollouts to repair metadata.
|
||||
pub use_state_db_only: bool,
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user