mirror of
https://github.com/openai/codex.git
synced 2026-04-28 10:21:06 +03:00
Add field to Thread object for the latest rename set for a given thread (#12301)
Exposes through the app server updated names set for a thread. This enables other surfaces to use the core as the source of truth for thread naming. `threadName` is gathered using the helper functions used to interact with `session_index.jsonl`, and is hydrated in: - `thread/list` - `thread/read` - `thread/resume` - `thread/unarchive` - `thread/rollback` We don't do this for `thread/start` and `thread/fork`.
This commit is contained in:
@@ -18,6 +18,7 @@ use codex_app_server_protocol::ThreadStatus;
|
||||
use codex_app_server_protocol::TurnStatus;
|
||||
use codex_app_server_protocol::UserInput;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use std::path::Path;
|
||||
use tempfile::TempDir;
|
||||
use tokio::time::timeout;
|
||||
@@ -70,8 +71,20 @@ async fn thread_fork_creates_new_thread_and_emits_started() -> Result<()> {
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(fork_id)),
|
||||
)
|
||||
.await??;
|
||||
let fork_result = fork_resp.result.clone();
|
||||
let ThreadForkResponse { thread, .. } = to_response::<ThreadForkResponse>(fork_resp)?;
|
||||
|
||||
// Wire contract: thread title field is `name`, serialized as null when unset.
|
||||
let thread_json = fork_result
|
||||
.get("thread")
|
||||
.and_then(Value::as_object)
|
||||
.expect("thread/fork result.thread must be an object");
|
||||
assert_eq!(
|
||||
thread_json.get("name"),
|
||||
Some(&Value::Null),
|
||||
"forked threads do not inherit a name; expected `name: null`"
|
||||
);
|
||||
|
||||
let after_contents = std::fs::read_to_string(&original_path)?;
|
||||
assert_eq!(
|
||||
after_contents, original_contents,
|
||||
@@ -87,6 +100,7 @@ async fn thread_fork_creates_new_thread_and_emits_started() -> Result<()> {
|
||||
assert_ne!(thread_path, original_path);
|
||||
assert!(thread.cwd.is_absolute());
|
||||
assert_eq!(thread.source, SessionSource::VsCode);
|
||||
assert_eq!(thread.name, None);
|
||||
|
||||
assert_eq!(
|
||||
thread.turns.len(),
|
||||
@@ -115,6 +129,16 @@ async fn thread_fork_creates_new_thread_and_emits_started() -> Result<()> {
|
||||
mcp.read_stream_until_notification_message("thread/started"),
|
||||
)
|
||||
.await??;
|
||||
let started_params = notif.params.clone().expect("params must be present");
|
||||
let started_thread_json = started_params
|
||||
.get("thread")
|
||||
.and_then(Value::as_object)
|
||||
.expect("thread/started params.thread must be an object");
|
||||
assert_eq!(
|
||||
started_thread_json.get("name"),
|
||||
Some(&Value::Null),
|
||||
"thread/started must serialize `name: null` when unset"
|
||||
);
|
||||
let started: ThreadStartedNotification =
|
||||
serde_json::from_value(notif.params.expect("params must be present"))?;
|
||||
assert_eq!(started.thread, thread);
|
||||
|
||||
@@ -8,8 +8,14 @@ use codex_app_server_protocol::JSONRPCResponse;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::SessionSource;
|
||||
use codex_app_server_protocol::ThreadItem;
|
||||
use codex_app_server_protocol::ThreadListParams;
|
||||
use codex_app_server_protocol::ThreadListResponse;
|
||||
use codex_app_server_protocol::ThreadReadParams;
|
||||
use codex_app_server_protocol::ThreadReadResponse;
|
||||
use codex_app_server_protocol::ThreadResumeParams;
|
||||
use codex_app_server_protocol::ThreadResumeResponse;
|
||||
use codex_app_server_protocol::ThreadSetNameParams;
|
||||
use codex_app_server_protocol::ThreadSetNameResponse;
|
||||
use codex_app_server_protocol::ThreadStartParams;
|
||||
use codex_app_server_protocol::ThreadStartResponse;
|
||||
use codex_app_server_protocol::ThreadStatus;
|
||||
@@ -21,6 +27,7 @@ use codex_protocol::user_input::ByteRange;
|
||||
use codex_protocol::user_input::TextElement;
|
||||
use core_test_support::responses;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use tempfile::TempDir;
|
||||
@@ -192,6 +199,155 @@ async fn thread_read_loaded_thread_returns_precomputed_path_before_materializati
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_name_set_is_reflected_in_read_list_and_resume() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri())?;
|
||||
|
||||
let preview = "Saved user message";
|
||||
let conversation_id = create_fake_rollout_with_text_elements(
|
||||
codex_home.path(),
|
||||
"2025-01-05T12-00-00",
|
||||
"2025-01-05T12:00:00Z",
|
||||
preview,
|
||||
vec![],
|
||||
Some("mock_provider"),
|
||||
None,
|
||||
)?;
|
||||
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
// `thread/name/set` operates on loaded threads (via ThreadManager). A rollout existing on disk
|
||||
// is not enough; we must `thread/resume` first to load it into the running server.
|
||||
let pre_resume_id = mcp
|
||||
.send_thread_resume_request(ThreadResumeParams {
|
||||
thread_id: conversation_id.clone(),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let pre_resume_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(pre_resume_id)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadResumeResponse {
|
||||
thread: pre_resumed,
|
||||
..
|
||||
} = to_response::<ThreadResumeResponse>(pre_resume_resp)?;
|
||||
assert_eq!(pre_resumed.id, conversation_id);
|
||||
|
||||
// Set a user-facing thread title.
|
||||
let new_name = "My renamed thread";
|
||||
let set_id = mcp
|
||||
.send_thread_set_name_request(ThreadSetNameParams {
|
||||
thread_id: conversation_id.clone(),
|
||||
name: new_name.to_string(),
|
||||
})
|
||||
.await?;
|
||||
let set_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(set_id)),
|
||||
)
|
||||
.await??;
|
||||
let _: ThreadSetNameResponse = to_response::<ThreadSetNameResponse>(set_resp)?;
|
||||
|
||||
// Read should now surface `thread.name`, and the wire payload must include `name`.
|
||||
let read_id = mcp
|
||||
.send_thread_read_request(ThreadReadParams {
|
||||
thread_id: conversation_id.clone(),
|
||||
include_turns: false,
|
||||
})
|
||||
.await?;
|
||||
let read_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(read_id)),
|
||||
)
|
||||
.await??;
|
||||
let read_result = read_resp.result.clone();
|
||||
let ThreadReadResponse { thread } = to_response::<ThreadReadResponse>(read_resp)?;
|
||||
assert_eq!(thread.id, conversation_id);
|
||||
assert_eq!(thread.name.as_deref(), Some(new_name));
|
||||
let thread_json = read_result
|
||||
.get("thread")
|
||||
.and_then(Value::as_object)
|
||||
.expect("thread/read result.thread must be an object");
|
||||
assert_eq!(
|
||||
thread_json.get("name").and_then(Value::as_str),
|
||||
Some(new_name),
|
||||
"thread/read must serialize `thread.name` on the wire"
|
||||
);
|
||||
|
||||
// List should also surface the name.
|
||||
let list_id = mcp
|
||||
.send_thread_list_request(ThreadListParams {
|
||||
cursor: None,
|
||||
limit: Some(50),
|
||||
sort_key: None,
|
||||
model_providers: Some(vec!["mock_provider".to_string()]),
|
||||
source_kinds: None,
|
||||
archived: None,
|
||||
cwd: None,
|
||||
})
|
||||
.await?;
|
||||
let list_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(list_id)),
|
||||
)
|
||||
.await??;
|
||||
let list_result = list_resp.result.clone();
|
||||
let ThreadListResponse { data, .. } = to_response::<ThreadListResponse>(list_resp)?;
|
||||
let listed = data
|
||||
.iter()
|
||||
.find(|t| t.id == conversation_id)
|
||||
.expect("thread/list should include the created thread");
|
||||
assert_eq!(listed.name.as_deref(), Some(new_name));
|
||||
let listed_json = list_result
|
||||
.get("data")
|
||||
.and_then(Value::as_array)
|
||||
.expect("thread/list result.data must be an array")
|
||||
.iter()
|
||||
.find(|t| t.get("id").and_then(Value::as_str) == Some(&conversation_id))
|
||||
.and_then(Value::as_object)
|
||||
.expect("thread/list should include the created thread as an object");
|
||||
assert_eq!(
|
||||
listed_json.get("name").and_then(Value::as_str),
|
||||
Some(new_name),
|
||||
"thread/list must serialize `thread.name` on the wire"
|
||||
);
|
||||
|
||||
// Resume should also surface the name.
|
||||
let resume_id = mcp
|
||||
.send_thread_resume_request(ThreadResumeParams {
|
||||
thread_id: conversation_id.clone(),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let resume_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(resume_id)),
|
||||
)
|
||||
.await??;
|
||||
let resume_result = resume_resp.result.clone();
|
||||
let ThreadResumeResponse {
|
||||
thread: resumed, ..
|
||||
} = to_response::<ThreadResumeResponse>(resume_resp)?;
|
||||
assert_eq!(resumed.id, conversation_id);
|
||||
assert_eq!(resumed.name.as_deref(), Some(new_name));
|
||||
let resumed_json = resume_result
|
||||
.get("thread")
|
||||
.and_then(Value::as_object)
|
||||
.expect("thread/resume result.thread must be an object");
|
||||
assert_eq!(
|
||||
resumed_json.get("name").and_then(Value::as_str),
|
||||
Some(new_name),
|
||||
"thread/resume must serialize `thread.name` on the wire"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_read_include_turns_rejects_unmaterialized_loaded_thread() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
|
||||
@@ -16,6 +16,7 @@ use codex_app_server_protocol::ThreadStatus;
|
||||
use codex_app_server_protocol::TurnStartParams;
|
||||
use codex_app_server_protocol::UserInput as V2UserInput;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use tempfile::TempDir;
|
||||
use tokio::time::timeout;
|
||||
|
||||
@@ -107,10 +108,23 @@ async fn thread_rollback_drops_last_turns_and_persists_to_rollout() -> Result<()
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(rollback_id)),
|
||||
)
|
||||
.await??;
|
||||
let rollback_result = rollback_resp.result.clone();
|
||||
let ThreadRollbackResponse {
|
||||
thread: rolled_back_thread,
|
||||
} = to_response::<ThreadRollbackResponse>(rollback_resp)?;
|
||||
|
||||
// Wire contract: thread title field is `name`, serialized as null when unset.
|
||||
let thread_json = rollback_result
|
||||
.get("thread")
|
||||
.and_then(Value::as_object)
|
||||
.expect("thread/rollback result.thread must be an object");
|
||||
assert_eq!(rolled_back_thread.name, None);
|
||||
assert_eq!(
|
||||
thread_json.get("name"),
|
||||
Some(&Value::Null),
|
||||
"thread/rollback must serialize `name: null` when unset"
|
||||
);
|
||||
|
||||
assert_eq!(rolled_back_thread.turns.len(), 1);
|
||||
assert_eq!(rolled_back_thread.status, ThreadStatus::Idle);
|
||||
assert_eq!(rolled_back_thread.turns[0].items.len(), 2);
|
||||
|
||||
@@ -13,6 +13,7 @@ use codex_app_server_protocol::ThreadStatus;
|
||||
use codex_core::config::set_project_trust_level;
|
||||
use codex_protocol::config_types::TrustLevel;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use serde_json::Value;
|
||||
use std::path::Path;
|
||||
use tempfile::TempDir;
|
||||
use tokio::time::timeout;
|
||||
@@ -45,6 +46,7 @@ async fn thread_start_creates_thread_and_emits_started() -> Result<()> {
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(req_id)),
|
||||
)
|
||||
.await??;
|
||||
let resp_result = resp.result.clone();
|
||||
let ThreadStartResponse {
|
||||
thread,
|
||||
model_provider,
|
||||
@@ -68,12 +70,34 @@ async fn thread_start_creates_thread_and_emits_started() -> Result<()> {
|
||||
"fresh thread rollout should not be materialized until first user message"
|
||||
);
|
||||
|
||||
// Wire contract: thread title field is `name`, serialized as null when unset.
|
||||
let thread_json = resp_result
|
||||
.get("thread")
|
||||
.and_then(Value::as_object)
|
||||
.expect("thread/start result.thread must be an object");
|
||||
assert_eq!(
|
||||
thread_json.get("name"),
|
||||
Some(&Value::Null),
|
||||
"new threads should serialize `name: null`"
|
||||
);
|
||||
assert_eq!(thread.name, None);
|
||||
|
||||
// A corresponding thread/started notification should arrive.
|
||||
let notif: JSONRPCNotification = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("thread/started"),
|
||||
)
|
||||
.await??;
|
||||
let started_params = notif.params.clone().expect("params must be present");
|
||||
let started_thread_json = started_params
|
||||
.get("thread")
|
||||
.and_then(Value::as_object)
|
||||
.expect("thread/started params.thread must be an object");
|
||||
assert_eq!(
|
||||
started_thread_json.get("name"),
|
||||
Some(&Value::Null),
|
||||
"thread/started should serialize `name: null` for new threads"
|
||||
);
|
||||
let started: ThreadStartedNotification =
|
||||
serde_json::from_value(notif.params.expect("params must be present"))?;
|
||||
assert_eq!(started.thread, thread);
|
||||
|
||||
@@ -18,6 +18,7 @@ use codex_app_server_protocol::UserInput;
|
||||
use codex_core::find_archived_thread_path_by_id_str;
|
||||
use codex_core::find_thread_path_by_id_str;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use std::fs::FileTimes;
|
||||
use std::fs::OpenOptions;
|
||||
use std::path::Path;
|
||||
@@ -120,6 +121,7 @@ async fn thread_unarchive_moves_rollout_back_into_sessions_directory() -> Result
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(unarchive_id)),
|
||||
)
|
||||
.await??;
|
||||
let unarchive_result = unarchive_resp.result.clone();
|
||||
let ThreadUnarchiveResponse {
|
||||
thread: unarchived_thread,
|
||||
} = to_response::<ThreadUnarchiveResponse>(unarchive_resp)?;
|
||||
@@ -140,6 +142,18 @@ async fn thread_unarchive_moves_rollout_back_into_sessions_directory() -> Result
|
||||
);
|
||||
assert_eq!(unarchived_thread.status, ThreadStatus::NotLoaded);
|
||||
|
||||
// Wire contract: thread title field is `name`, serialized as null when unset.
|
||||
let thread_json = unarchive_result
|
||||
.get("thread")
|
||||
.and_then(Value::as_object)
|
||||
.expect("thread/unarchive result.thread must be an object");
|
||||
assert_eq!(unarchived_thread.name, None);
|
||||
assert_eq!(
|
||||
thread_json.get("name"),
|
||||
Some(&Value::Null),
|
||||
"thread/unarchive must serialize `name: null` when unset"
|
||||
);
|
||||
|
||||
let rollout_path_display = rollout_path.display();
|
||||
assert!(
|
||||
rollout_path.exists(),
|
||||
|
||||
Reference in New Issue
Block a user