From a271bc8fba0f79213763c3d563043cbab0d99915 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 18 Nov 2025 16:29:40 -0800 Subject: [PATCH] fix: add more fields to ThreadStartResponse and ThreadResumeResponse --- .../app-server-protocol/src/protocol/v2.rs | 13 ++++- .../app-server/src/codex_message_processor.rs | 49 ++++++++++++++----- .../tests/suite/v2/thread_archive.rs | 2 +- .../app-server/tests/suite/v2/thread_list.rs | 3 -- .../tests/suite/v2/thread_resume.rs | 25 ++++++---- .../app-server/tests/suite/v2/thread_start.rs | 8 ++- .../tests/suite/v2/turn_interrupt.rs | 2 +- .../app-server/tests/suite/v2/turn_start.rs | 8 +-- 8 files changed, 76 insertions(+), 34 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index a2b9cee3fe..372873ee94 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -402,6 +402,12 @@ pub struct ThreadStartParams { #[ts(export_to = "v2/")] pub struct ThreadStartResponse { pub thread: Thread, + pub model: String, + pub model_provider: String, + pub cwd: PathBuf, + pub approval_policy: AskForApproval, + pub sandbox: SandboxPolicy, + pub reasoning_effort: Option, } #[derive(Serialize, Deserialize, Debug, Default, Clone, PartialEq, JsonSchema, TS)] @@ -444,6 +450,12 @@ pub struct ThreadResumeParams { #[ts(export_to = "v2/")] pub struct ThreadResumeResponse { pub thread: Thread, + pub model: String, + pub model_provider: String, + pub cwd: PathBuf, + pub approval_policy: AskForApproval, + pub sandbox: SandboxPolicy, + pub reasoning_effort: Option, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] @@ -500,7 +512,6 @@ pub struct Thread { pub id: String, /// Usually the first user message in the thread, if available. pub preview: String, - pub model_provider: String, /// Unix timestamp (in seconds) when the thread was created. pub created_at: i64, /// [UNSTABLE] Path to the thread on disk. diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index c5fa2a7fa2..5f392f5c66 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -1210,10 +1210,20 @@ impl CodexMessageProcessor { } }; + let model = config.model.clone(); + let model_provider = config.model_provider_id.clone(); + let cwd = config.cwd.clone(); + let approval_policy = config.approval_policy; + let sandbox_policy = config.sandbox_policy.clone(); + match self.conversation_manager.new_conversation(config).await { Ok(new_conv) => { - let conversation_id = new_conv.conversation_id; - let rollout_path = new_conv.session_configured.rollout_path.clone(); + let NewConversation { + conversation_id, + session_configured, + .. + } = new_conv; + let rollout_path = session_configured.rollout_path.clone(); let fallback_provider = self.config.model_provider_id.as_str(); // A bit hacky, but the summary contains a lot of useful information for the thread @@ -1224,7 +1234,7 @@ impl CodexMessageProcessor { ) .await { - Ok(summary) => summary_to_thread(summary), + Ok(summary) => summary_to_thread(&summary), Err(err) => { self.send_internal_error( request_id, @@ -1240,6 +1250,12 @@ impl CodexMessageProcessor { let response = ThreadStartResponse { thread: thread.clone(), + model, + model_provider, + cwd, + approval_policy: approval_policy.into(), + sandbox: sandbox_policy.into(), + reasoning_effort: session_configured.reasoning_effort, }; // Auto-attach a conversation listener when starting a thread. @@ -1373,7 +1389,7 @@ impl CodexMessageProcessor { } }; - let data = summaries.into_iter().map(summary_to_thread).collect(); + let data = summaries.iter().map(summary_to_thread).collect(); let response = ThreadListResponse { data, next_cursor }; self.outgoing.send_response(request_id, response).await; @@ -1506,6 +1522,8 @@ impl CodexMessageProcessor { }; let fallback_model_provider = config.model_provider_id.clone(); + let approval_policy = config.approval_policy; + let sandbox_policy = config.sandbox_policy.clone(); match self .conversation_manager @@ -1533,13 +1551,13 @@ impl CodexMessageProcessor { ); } - let thread = match read_summary_from_rollout( + let summary = match read_summary_from_rollout( session_configured.rollout_path.as_path(), fallback_model_provider.as_str(), ) .await { - Ok(summary) => summary_to_thread(summary), + Ok(summary) => summary, Err(err) => { self.send_internal_error( request_id, @@ -1552,7 +1570,16 @@ impl CodexMessageProcessor { return; } }; - let response = ThreadResumeResponse { thread }; + let thread = summary_to_thread(&summary); + let response = ThreadResumeResponse { + thread, + model: session_configured.model, + model_provider: summary.model_provider, + cwd: summary.cwd, + approval_policy: approval_policy.into(), + sandbox: sandbox_policy.into(), + reasoning_effort: session_configured.reasoning_effort, + }; self.outgoing.send_response(request_id, response).await; } Err(err) => { @@ -2773,13 +2800,12 @@ fn parse_datetime(timestamp: Option<&str>) -> Option> { }) } -fn summary_to_thread(summary: ConversationSummary) -> Thread { +fn summary_to_thread(summary: &ConversationSummary) -> Thread { let ConversationSummary { conversation_id, path, preview, timestamp, - model_provider, .. } = summary; @@ -2787,10 +2813,9 @@ fn summary_to_thread(summary: ConversationSummary) -> Thread { Thread { id: conversation_id.to_string(), - preview, - model_provider, + preview: preview.clone(), created_at: created_at.map(|dt| dt.timestamp()).unwrap_or(0), - path, + path: path.clone(), } } diff --git a/codex-rs/app-server/tests/suite/v2/thread_archive.rs b/codex-rs/app-server/tests/suite/v2/thread_archive.rs index 083f3da901..88891af77d 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_archive.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_archive.rs @@ -35,7 +35,7 @@ async fn thread_archive_moves_rollout_into_archived_directory() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(start_id)), ) .await??; - let ThreadStartResponse { thread } = to_response::(start_resp)?; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; assert!(!thread.id.is_empty()); // Locate the rollout path recorded for this thread id. diff --git a/codex-rs/app-server/tests/suite/v2/thread_list.rs b/codex-rs/app-server/tests/suite/v2/thread_list.rs index 464fb4eee8..f87a3b1fc6 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_list.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_list.rs @@ -102,7 +102,6 @@ async fn thread_list_pagination_next_cursor_none_on_last_page() -> Result<()> { assert_eq!(data1.len(), 2); for thread in &data1 { assert_eq!(thread.preview, "Hello"); - assert_eq!(thread.model_provider, "mock_provider"); assert!(thread.created_at > 0); } let cursor1 = cursor1.expect("expected nextCursor on first page"); @@ -127,7 +126,6 @@ async fn thread_list_pagination_next_cursor_none_on_last_page() -> Result<()> { assert!(data2.len() <= 2); for thread in &data2 { assert_eq!(thread.preview, "Hello"); - assert_eq!(thread.model_provider, "mock_provider"); assert!(thread.created_at > 0); } assert_eq!(cursor2, None, "expected nextCursor to be null on last page"); @@ -177,7 +175,6 @@ async fn thread_list_respects_provider_filter() -> Result<()> { assert_eq!(next_cursor, None); let thread = &data[0]; assert_eq!(thread.preview, "X"); - assert_eq!(thread.model_provider, "other_provider"); let expected_ts = chrono::DateTime::parse_from_rfc3339("2025-01-02T11:00:00Z")?.timestamp(); assert_eq!(thread.created_at, expected_ts); diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index bda2d14172..89020c8876 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -36,7 +36,7 @@ async fn thread_resume_returns_original_thread() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(start_id)), ) .await??; - let ThreadStartResponse { thread } = to_response::(start_resp)?; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; // Resume it via v2 API. let resume_id = mcp @@ -50,8 +50,9 @@ async fn thread_resume_returns_original_thread() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(resume_id)), ) .await??; - let ThreadResumeResponse { thread: resumed } = - to_response::(resume_resp)?; + let ThreadResumeResponse { + thread: resumed, .. + } = to_response::(resume_resp)?; assert_eq!(resumed, thread); Ok(()) @@ -77,7 +78,7 @@ async fn thread_resume_prefers_path_over_thread_id() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(start_id)), ) .await??; - let ThreadStartResponse { thread } = to_response::(start_resp)?; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; let thread_path = thread.path.clone(); let resume_id = mcp @@ -93,8 +94,9 @@ async fn thread_resume_prefers_path_over_thread_id() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(resume_id)), ) .await??; - let ThreadResumeResponse { thread: resumed } = - to_response::(resume_resp)?; + let ThreadResumeResponse { + thread: resumed, .. + } = to_response::(resume_resp)?; assert_eq!(resumed, thread); Ok(()) @@ -121,7 +123,7 @@ async fn thread_resume_supports_history_and_overrides() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(start_id)), ) .await??; - let ThreadStartResponse { thread } = to_response::(start_resp)?; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; let history_text = "Hello from history"; let history = vec![ResponseItem::Message { @@ -147,10 +149,13 @@ async fn thread_resume_supports_history_and_overrides() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(resume_id)), ) .await??; - let ThreadResumeResponse { thread: resumed } = - to_response::(resume_resp)?; + let ThreadResumeResponse { + thread: resumed, + model_provider, + .. + } = to_response::(resume_resp)?; assert!(!resumed.id.is_empty()); - assert_eq!(resumed.model_provider, "mock_provider"); + assert_eq!(model_provider, "mock_provider"); assert_eq!(resumed.preview, history_text); Ok(()) diff --git a/codex-rs/app-server/tests/suite/v2/thread_start.rs b/codex-rs/app-server/tests/suite/v2/thread_start.rs index a5e4c0d487..ad0949ba29 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -40,13 +40,17 @@ async fn thread_start_creates_thread_and_emits_started() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(req_id)), ) .await??; - let ThreadStartResponse { thread } = to_response::(resp)?; + let ThreadStartResponse { + thread, + model_provider, + .. + } = to_response::(resp)?; assert!(!thread.id.is_empty(), "thread id should not be empty"); assert!( thread.preview.is_empty(), "new threads should start with an empty preview" ); - assert_eq!(thread.model_provider, "mock_provider"); + assert_eq!(model_provider, "mock_provider"); assert!( thread.created_at > 0, "created_at should be a positive UNIX timestamp" diff --git a/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs b/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs index d1deb60801..34b3cc8ecd 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs @@ -62,7 +62,7 @@ async fn turn_interrupt_aborts_running_turn() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(thread_req)), ) .await??; - let ThreadStartResponse { thread } = to_response::(thread_resp)?; + let ThreadStartResponse { thread, .. } = to_response::(thread_resp)?; // Start a turn that triggers a long-running command. let turn_req = mcp diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index 433c7b4486..71e3f2904a 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -57,7 +57,7 @@ async fn turn_start_emits_notifications_and_accepts_model_override() -> Result<( mcp.read_stream_until_response_message(RequestId::Integer(thread_req)), ) .await??; - let ThreadStartResponse { thread } = to_response::(thread_resp)?; + let ThreadStartResponse { thread, .. } = to_response::(thread_resp)?; // Start a turn with only input and thread_id set (no overrides). let turn_req = mcp @@ -157,7 +157,7 @@ async fn turn_start_accepts_local_image_input() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(thread_req)), ) .await??; - let ThreadStartResponse { thread } = to_response::(thread_resp)?; + let ThreadStartResponse { thread, .. } = to_response::(thread_resp)?; let image_path = codex_home.path().join("image.png"); // No need to actually write the file; we just exercise the input path. @@ -233,7 +233,7 @@ async fn turn_start_exec_approval_toggle_v2() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(start_id)), ) .await??; - let ThreadStartResponse { thread } = to_response::(start_resp)?; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; // turn/start — expect CommandExecutionRequestApproval request from server let first_turn_id = mcp @@ -362,7 +362,7 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(start_id)), ) .await??; - let ThreadStartResponse { thread } = to_response::(start_resp)?; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; // first turn with workspace-write sandbox and first_cwd let first_turn = mcp