mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
Persist thread section moves before the first turn (#39523)
## Why New non-ephemeral threads have no persisted rollout or preview until their first turn, so moving them into a section could leave them absent from section-filtered thread lists. ## What changed - Materialize and flush a non-ephemeral thread before applying an explicit section move. - Include explicitly sectioned threads with empty previews in section-filtered queries, and update the partial indexes to cover them. ## Testing - Add an app-server test that moves a newly started thread into and out of the pinned section before its first turn. - Cover empty-preview threads in recency and section-position query tests. GitOrigin-RevId: aa3c4daf9890f06c08b93cd60f146099cbc6cfb2
This commit is contained in:
committed by
copyberry
parent
ffad922340
commit
1b450c7912
@@ -649,7 +649,7 @@ to load the containing turn.
|
||||
|
||||
### Example: Update stored thread metadata
|
||||
|
||||
Use `thread/metadata/update` to patch sqlite-backed `gitInfo` without resuming a thread. Omitted fields are left unchanged, while explicit `null` clears a stored value. Use `thread/section/move` to enter, reorder, or leave a section; section positions remain server-owned, and `thread/list` returns threads in their manual order when `sortKey` is `section_position`.
|
||||
Use `thread/metadata/update` to patch sqlite-backed `gitInfo` without resuming a thread. Omitted fields are left unchanged, while explicit `null` clears a stored value. Use `thread/section/move` to enter, reorder, or leave a section; an explicit move persists a newly started non-ephemeral thread even before its first turn. Section positions remain server-owned, and `thread/list` returns threads in their manual order when `sortKey` is `section_position`. A non-null `sectionId` filter includes explicitly placed threads whose preview is still empty.
|
||||
|
||||
```json
|
||||
{ "method": "thread/metadata/update", "id": 24, "params": {
|
||||
|
||||
@@ -48,6 +48,94 @@ use tokio::time::timeout;
|
||||
const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
|
||||
const INVALID_REQUEST_ERROR_CODE: i64 = -32600;
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_section_move_pins_before_first_turn() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
mock_responses_config(&server.uri()).write(codex_home.path())?;
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.build()
|
||||
.await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let start_id = mcp
|
||||
.send_thread_start_request_with_auto_env(ThreadStartParams {
|
||||
model: Some("mock-model".to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let started: ThreadStartResponse =
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(start_id)).await??;
|
||||
let thread_id = started.thread.id;
|
||||
assert_eq!(started.thread.preview, "");
|
||||
|
||||
let move_id = mcp
|
||||
.send_thread_section_move_request(ThreadSectionMoveParams {
|
||||
thread_id: thread_id.clone(),
|
||||
section_id: Some(PINNED_THREAD_SECTION_ID.to_string()),
|
||||
before_thread_id: None,
|
||||
})
|
||||
.await?;
|
||||
let _: ThreadSectionMoveResponse =
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(move_id)).await??;
|
||||
|
||||
let list_params = ThreadListParams {
|
||||
cursor: None,
|
||||
limit: Some(100),
|
||||
sort_key: None,
|
||||
sort_direction: None,
|
||||
model_providers: None,
|
||||
source_kinds: None,
|
||||
archived: None,
|
||||
section_id: Some(Some(PINNED_THREAD_SECTION_ID.to_string())),
|
||||
project_id: None,
|
||||
cwd: None,
|
||||
use_state_db_only: true,
|
||||
search_term: None,
|
||||
parent_thread_id: None,
|
||||
ancestor_thread_id: None,
|
||||
};
|
||||
for sort_key in [ThreadSortKey::SectionPosition, ThreadSortKey::RecencyAt] {
|
||||
let list_id = mcp
|
||||
.send_thread_list_request(ThreadListParams {
|
||||
sort_key: Some(sort_key),
|
||||
..list_params.clone()
|
||||
})
|
||||
.await?;
|
||||
let listed: ThreadListResponse =
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(list_id)).await??;
|
||||
assert_eq!(
|
||||
listed
|
||||
.data
|
||||
.iter()
|
||||
.map(|thread| thread.id.as_str())
|
||||
.collect::<Vec<_>>(),
|
||||
vec![thread_id.as_str()]
|
||||
);
|
||||
}
|
||||
|
||||
let move_id = mcp
|
||||
.send_thread_section_move_request(ThreadSectionMoveParams {
|
||||
thread_id,
|
||||
section_id: None,
|
||||
before_thread_id: None,
|
||||
})
|
||||
.await?;
|
||||
let _: ThreadSectionMoveResponse =
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(move_id)).await??;
|
||||
let list_id = mcp
|
||||
.send_thread_list_request(ThreadListParams {
|
||||
section_id: None,
|
||||
..list_params
|
||||
})
|
||||
.await?;
|
||||
let listed: ThreadListResponse =
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(list_id)).await??;
|
||||
assert_eq!(listed.data, vec![]);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_section_move_pins_and_unpins_with_filtered_recency_pagination() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
|
||||
@@ -835,19 +835,22 @@ impl ThreadManager {
|
||||
}
|
||||
}
|
||||
|
||||
/// Moves a persisted thread to, within, or out of a server-ordered section.
|
||||
/// Moves a thread to, within, or out of a server-ordered section.
|
||||
pub async fn move_thread_to_section(
|
||||
&self,
|
||||
thread_id: ThreadId,
|
||||
section: Option<&str>,
|
||||
before_thread_id: Option<ThreadId>,
|
||||
) -> CodexResult<()> {
|
||||
if let Ok(thread) = self.get_thread(thread_id).await
|
||||
&& thread.config_snapshot().await.ephemeral
|
||||
{
|
||||
return Err(CodexErr::InvalidRequest(format!(
|
||||
"ephemeral thread does not support section moves: {thread_id}"
|
||||
)));
|
||||
if let Ok(thread) = self.get_thread(thread_id).await {
|
||||
if thread.config_snapshot().await.ephemeral {
|
||||
return Err(CodexErr::InvalidRequest(format!(
|
||||
"ephemeral thread does not support section moves: {thread_id}"
|
||||
)));
|
||||
}
|
||||
// Explicit placement must work before the first turn materializes the thread.
|
||||
thread.ensure_rollout_materialized().await;
|
||||
thread.flush_rollout().await?;
|
||||
}
|
||||
|
||||
self.state
|
||||
|
||||
@@ -0,0 +1,10 @@
|
||||
DROP INDEX idx_threads_section_recency_at_ms;
|
||||
DROP INDEX idx_threads_section_position;
|
||||
|
||||
CREATE INDEX idx_threads_section_recency_at_ms
|
||||
ON threads(archived, thread_section_id, recency_at_ms DESC, id DESC)
|
||||
WHERE thread_section_id IS NOT NULL;
|
||||
|
||||
CREATE INDEX idx_threads_section_position
|
||||
ON threads(archived, thread_section_id, section_position ASC, id ASC)
|
||||
WHERE thread_section_id IS NOT NULL;
|
||||
@@ -1382,7 +1382,7 @@ fn push_thread_filters_with_preview<'a>(
|
||||
} else {
|
||||
builder.push(" AND threads.archived = 0");
|
||||
}
|
||||
if !include_empty_preview {
|
||||
if !include_empty_preview && !matches!(section, Some(Some(_))) {
|
||||
builder.push(" AND threads.preview <> ''");
|
||||
}
|
||||
match section {
|
||||
@@ -1674,6 +1674,10 @@ mod tests {
|
||||
] {
|
||||
let mut metadata = test_thread_metadata(&codex_home, thread_id, codex_home.clone());
|
||||
metadata.recency_at = DateTime::<Utc>::from_timestamp(recency_at, 0).unwrap();
|
||||
if thread_id == oldest_pinned {
|
||||
metadata.preview = Some(String::new());
|
||||
metadata.first_user_message = None;
|
||||
}
|
||||
metadata.section = section.map(|id| crate::ThreadSection {
|
||||
id: id.to_string(),
|
||||
name: crate::PINNED_THREAD_SECTION_NAME.to_string(),
|
||||
@@ -1748,12 +1752,7 @@ mod tests {
|
||||
.iter()
|
||||
.map(|thread| thread.id)
|
||||
.collect::<Vec<_>>(),
|
||||
vec![
|
||||
newest_unpinned,
|
||||
newest_pinned,
|
||||
oldest_pinned,
|
||||
oldest_unpinned,
|
||||
]
|
||||
vec![newest_unpinned, newest_pinned, oldest_unpinned,]
|
||||
);
|
||||
|
||||
let mut builder = QueryBuilder::<Sqlite>::new("EXPLAIN QUERY PLAN ");
|
||||
@@ -1806,6 +1805,10 @@ mod tests {
|
||||
|
||||
for (thread_id, position) in [(first, 1_000_000), (tied, 1_000_000), (last, 2_000_000)] {
|
||||
let mut metadata = test_thread_metadata(&codex_home, thread_id, codex_home.clone());
|
||||
if thread_id == tied {
|
||||
metadata.preview = Some(String::new());
|
||||
metadata.first_user_message = None;
|
||||
}
|
||||
metadata.section = Some(crate::ThreadSection {
|
||||
id: CUSTOM_THREAD_SECTION_ID.to_string(),
|
||||
name: "Custom section".to_string(),
|
||||
|
||||
Reference in New Issue
Block a user