fix: add more fields to ThreadStartResponse and ThreadResumeResponse (#6847)

This adds the following fields to `ThreadStartResponse` and
`ThreadResumeResponse`:

```rust
    pub model: String,
    pub model_provider: String,
    pub cwd: PathBuf,
    pub approval_policy: AskForApproval,
    pub sandbox: SandboxPolicy,
    pub reasoning_effort: Option<ReasoningEffort>,
```

This is important because these fields are optional in
`ThreadStartParams` and `ThreadResumeParams`, so the caller needs to be
able to determine what values were ultimately used to start/resume the
conversation. (Though note that any of these could be changed later
between turns in the conversation.)

Though to get this information reliably, it must be read from the
internal `SessionConfiguredEvent` that is created in response to the
start of a conversation. Because `SessionConfiguredEvent` (as defined in
`codex-rs/protocol/src/protocol.rs`) did not have all of these fields, a
number of them had to be added as part of this PR.

Because `SessionConfiguredEvent` is referenced in many tests, test
instances of `SessionConfiguredEvent` had to be updated, as well, which
is why this PR touches so many files.
This commit is contained in:
Michael Bolin
2025-11-18 21:18:43 -08:00
committed by GitHub
Unverified
parent 7508e4fd2d
commit a75321a64c
18 changed files with 139 additions and 37 deletions
+1
View File
@@ -1420,6 +1420,7 @@ dependencies = [
"icu_provider",
"mcp-types",
"mime_guess",
"pretty_assertions",
"schemars 0.8.22",
"serde",
"serde_json",
@@ -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<ReasoningEffort>,
}
#[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<ReasoningEffort>,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
@@ -118,6 +118,7 @@ use codex_core::parse_cursor;
use codex_core::protocol::EventMsg;
use codex_core::protocol::Op;
use codex_core::protocol::ReviewRequest;
use codex_core::protocol::SessionConfiguredEvent;
use codex_core::read_head_for_summary;
use codex_feedback::CodexFeedback;
use codex_login::ServerOptions as LoginServerOptions;
@@ -1313,8 +1314,12 @@ impl CodexMessageProcessor {
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
@@ -1339,8 +1344,22 @@ impl CodexMessageProcessor {
}
};
let SessionConfiguredEvent {
model,
model_provider_id,
cwd,
approval_policy,
sandbox_policy,
..
} = session_configured;
let response = ThreadStartResponse {
thread: thread.clone(),
model,
model_provider: model_provider_id,
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.
@@ -1653,7 +1672,15 @@ impl CodexMessageProcessor {
return;
}
};
let response = ThreadResumeResponse { thread };
let response = ThreadResumeResponse {
thread,
model: session_configured.model,
model_provider: session_configured.model_provider_id,
cwd: session_configured.cwd,
approval_policy: session_configured.approval_policy.into(),
sandbox: session_configured.sandbox_policy.into(),
reasoning_effort: session_configured.reasoning_effort,
};
self.outgoing.send_response(request_id, response).await;
}
Err(err) => {
+1 -1
View File
@@ -251,7 +251,7 @@ async fn start_default_thread(mcp: &mut McpProcess) -> Result<String> {
mcp.read_stream_until_response_message(RequestId::Integer(thread_req)),
)
.await??;
let ThreadStartResponse { thread } = to_response::<ThreadStartResponse>(thread_resp)?;
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(thread_resp)?;
Ok(thread.id)
}
@@ -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::<ThreadStartResponse>(start_resp)?;
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(start_resp)?;
assert!(!thread.id.is_empty());
// Locate the rollout path recorded for this thread id.
@@ -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::<ThreadStartResponse>(start_resp)?;
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(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::<ThreadResumeResponse>(resume_resp)?;
let ThreadResumeResponse {
thread: resumed, ..
} = to_response::<ThreadResumeResponse>(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::<ThreadStartResponse>(start_resp)?;
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(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::<ThreadResumeResponse>(resume_resp)?;
let ThreadResumeResponse {
thread: resumed, ..
} = to_response::<ThreadResumeResponse>(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::<ThreadStartResponse>(start_resp)?;
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(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::<ThreadResumeResponse>(resume_resp)?;
let ThreadResumeResponse {
thread: resumed,
model_provider,
..
} = to_response::<ThreadResumeResponse>(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(())
@@ -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::<ThreadStartResponse>(resp)?;
let ThreadStartResponse {
thread,
model_provider,
..
} = to_response::<ThreadStartResponse>(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"
@@ -65,7 +65,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::<ThreadStartResponse>(thread_resp)?;
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(thread_resp)?;
// Start a turn that triggers a long-running command.
let turn_req = mcp
@@ -59,7 +59,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::<ThreadStartResponse>(thread_resp)?;
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(thread_resp)?;
// Start a turn with only input and thread_id set (no overrides).
let turn_req = mcp
@@ -163,7 +163,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::<ThreadStartResponse>(thread_resp)?;
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(thread_resp)?;
let image_path = codex_home.path().join("image.png");
// No need to actually write the file; we just exercise the input path.
@@ -239,7 +239,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::<ThreadStartResponse>(start_resp)?;
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(start_resp)?;
// turn/start — expect CommandExecutionRequestApproval request from server
let first_turn_id = mcp
@@ -378,7 +378,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::<ThreadStartResponse>(start_resp)?;
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(start_resp)?;
// first turn with workspace-write sandbox and first_cwd
let first_turn = mcp
+4 -1
View File
@@ -293,7 +293,6 @@ impl TurnContext {
}
}
#[allow(dead_code)]
#[derive(Clone)]
pub(crate) struct SessionConfiguration {
/// Provider identifier ("openai", "openrouter", ...).
@@ -581,6 +580,10 @@ impl Session {
msg: EventMsg::SessionConfigured(SessionConfiguredEvent {
session_id: conversation_id,
model: session_configuration.model.clone(),
model_provider_id: config.model_provider_id.clone(),
approval_policy: session_configuration.approval_policy,
sandbox_policy: session_configuration.sandbox_policy.clone(),
cwd: session_configuration.cwd.clone(),
reasoning_effort: session_configuration.model_reasoning_effort,
history_log_id,
history_entry_count,
@@ -480,11 +480,7 @@ impl EventProcessor for EventProcessorWithHumanOutput {
let SessionConfiguredEvent {
session_id: conversation_id,
model,
reasoning_effort: _,
history_log_id: _,
history_entry_count: _,
initial_messages: _,
rollout_path: _,
..
} = session_configured_event;
ts_msg!(
@@ -1,5 +1,6 @@
use codex_core::protocol::AgentMessageEvent;
use codex_core::protocol::AgentReasoningEvent;
use codex_core::protocol::AskForApproval;
use codex_core::protocol::ErrorEvent;
use codex_core::protocol::Event;
use codex_core::protocol::EventMsg;
@@ -12,6 +13,7 @@ use codex_core::protocol::McpToolCallBeginEvent;
use codex_core::protocol::McpToolCallEndEvent;
use codex_core::protocol::PatchApplyBeginEvent;
use codex_core::protocol::PatchApplyEndEvent;
use codex_core::protocol::SandboxPolicy;
use codex_core::protocol::SessionConfiguredEvent;
use codex_core::protocol::WarningEvent;
use codex_core::protocol::WebSearchEndEvent;
@@ -72,6 +74,10 @@ fn session_configured_produces_thread_started_event() {
EventMsg::SessionConfigured(SessionConfiguredEvent {
session_id,
model: "codex-mini-latest".to_string(),
model_provider_id: "test-provider".to_string(),
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::ReadOnly,
cwd: PathBuf::from("/home/user/project"),
reasoning_effort: None,
history_log_id: 0,
history_entry_count: 0,
+20 -2
View File
@@ -231,8 +231,12 @@ pub(crate) struct OutgoingError {
#[cfg(test)]
mod tests {
use std::path::PathBuf;
use anyhow::Result;
use codex_core::protocol::AskForApproval;
use codex_core::protocol::EventMsg;
use codex_core::protocol::SandboxPolicy;
use codex_core::protocol::SessionConfiguredEvent;
use codex_protocol::ConversationId;
use codex_protocol::config_types::ReasoningEffort;
@@ -254,6 +258,10 @@ mod tests {
msg: EventMsg::SessionConfigured(SessionConfiguredEvent {
session_id: conversation_id,
model: "gpt-4o".to_string(),
model_provider_id: "test-provider".to_string(),
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::ReadOnly,
cwd: PathBuf::from("/home/user/project"),
reasoning_effort: Some(ReasoningEffort::default()),
history_log_id: 1,
history_entry_count: 1000,
@@ -289,6 +297,10 @@ mod tests {
let session_configured_event = SessionConfiguredEvent {
session_id: conversation_id,
model: "gpt-4o".to_string(),
model_provider_id: "test-provider".to_string(),
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::ReadOnly,
cwd: PathBuf::from("/home/user/project"),
reasoning_effort: Some(ReasoningEffort::default()),
history_log_id: 1,
history_entry_count: 1000,
@@ -318,12 +330,18 @@ mod tests {
},
"id": "1",
"msg": {
"type": "session_configured",
"session_id": session_configured_event.session_id,
"model": session_configured_event.model,
"model": "gpt-4o",
"model_provider_id": "test-provider",
"approval_policy": "never",
"sandbox_policy": {
"type": "read-only"
},
"cwd": "/home/user/project",
"reasoning_effort": session_configured_event.reasoning_effort,
"history_log_id": session_configured_event.history_log_id,
"history_entry_count": session_configured_event.history_entry_count,
"type": "session_configured",
"rollout_path": rollout_file.path().to_path_buf(),
}
});
+2 -1
View File
@@ -20,10 +20,10 @@ icu_locale_core = { workspace = true }
icu_provider = { workspace = true, features = ["sync"] }
mcp-types = { workspace = true }
mime_guess = { workspace = true }
schemars = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
serde_with = { workspace = true, features = ["macros", "base64"] }
schemars = { workspace = true }
strum = { workspace = true }
strum_macros = { workspace = true }
sys-locale = { workspace = true }
@@ -37,6 +37,7 @@ uuid = { workspace = true, features = ["serde", "v7", "v4"] }
[dev-dependencies]
anyhow = { workspace = true }
pretty_assertions = { workspace = true }
tempfile = { workspace = true }
[package.metadata.cargo-shear]
+24 -1
View File
@@ -1469,7 +1469,7 @@ pub struct ListCustomPromptsResponseEvent {
pub custom_prompts: Vec<CustomPrompt>,
}
#[derive(Debug, Default, Clone, Deserialize, Serialize, JsonSchema, TS)]
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
pub struct SessionConfiguredEvent {
/// Name left as session_id instead of conversation_id for backwards compatibility.
pub session_id: ConversationId,
@@ -1477,6 +1477,18 @@ pub struct SessionConfiguredEvent {
/// Tell the client what model is being queried.
pub model: String,
pub model_provider_id: String,
/// When to escalate for approval for execution
pub approval_policy: AskForApproval,
/// How to sandbox commands executed in the system
pub sandbox_policy: SandboxPolicy,
/// Working directory that should be treated as the *root* of the
/// session.
pub cwd: PathBuf,
/// The effort the model is putting into reasoning about the user's request.
#[serde(skip_serializing_if = "Option::is_none")]
pub reasoning_effort: Option<ReasoningEffortConfig>,
@@ -1562,6 +1574,7 @@ mod tests {
use crate::items::UserMessageItem;
use crate::items::WebSearchItem;
use anyhow::Result;
use pretty_assertions::assert_eq;
use serde_json::json;
use tempfile::NamedTempFile;
@@ -1606,6 +1619,10 @@ mod tests {
msg: EventMsg::SessionConfigured(SessionConfiguredEvent {
session_id: conversation_id,
model: "codex-mini-latest".to_string(),
model_provider_id: "openai".to_string(),
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::ReadOnly,
cwd: PathBuf::from("/home/user/project"),
reasoning_effort: Some(ReasoningEffortConfig::default()),
history_log_id: 0,
history_entry_count: 0,
@@ -1620,6 +1637,12 @@ mod tests {
"type": "session_configured",
"session_id": "67e55044-10b1-426f-9247-bb680e5fe0c8",
"model": "codex-mini-latest",
"model_provider_id": "openai",
"approval_policy": "never",
"sandbox_policy": {
"type": "read-only"
},
"cwd": "/home/user/project",
"reasoning_effort": "medium",
"history_log_id": 0,
"history_entry_count": 0,
+6
View File
@@ -934,6 +934,8 @@ mod tests {
use codex_core::AuthManager;
use codex_core::CodexAuth;
use codex_core::ConversationManager;
use codex_core::protocol::AskForApproval;
use codex_core::protocol::SandboxPolicy;
use codex_core::protocol::SessionConfiguredEvent;
use codex_protocol::ConversationId;
use ratatui::prelude::Line;
@@ -1043,6 +1045,10 @@ mod tests {
let event = SessionConfiguredEvent {
session_id: ConversationId::new(),
model: "gpt-test".to_string(),
model_provider_id: "test-provider".to_string(),
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::ReadOnly,
cwd: PathBuf::from("/home/user/project"),
reasoning_effort: None,
history_log_id: 0,
history_entry_count: 0,
+4
View File
@@ -93,6 +93,10 @@ fn resumed_initial_messages_render_history() {
let configured = codex_core::protocol::SessionConfiguredEvent {
session_id: conversation_id,
model: "test-model".to_string(),
model_provider_id: "test-provider".to_string(),
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::ReadOnly,
cwd: PathBuf::from("/home/user/project"),
reasoning_effort: Some(ReasoningEffortConfig::default()),
history_log_id: 0,
history_entry_count: 0,
+1 -5
View File
@@ -584,11 +584,7 @@ pub(crate) fn new_session_info(
let SessionConfiguredEvent {
model,
reasoning_effort,
session_id: _,
history_log_id: _,
history_entry_count: _,
initial_messages: _,
rollout_path: _,
..
} = event;
SessionInfoCell(if is_first_event {
// Header box rendered as history (so it appears at the very top)