fix: include rollout_path in NewConversationResponse (#3352)

Adding the `rollout_path` to the `NewConversationResponse` makes it so a
client can perform subsequent operations on a `(ConversationId,
PathBuf)` pair. #3353 will introduce support for `ArchiveConversation`.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/3352).
* #3353
* __->__ #3352
This commit is contained in:
Michael Bolin
2025-09-09 00:11:48 -07:00
committed by GitHub
parent 16309d6b68
commit 2a76a08a9e
14 changed files with 51 additions and 11 deletions

1
codex-rs/Cargo.lock generated
View File

@@ -830,6 +830,7 @@ dependencies = [
"strum 0.27.2", "strum 0.27.2",
"strum_macros 0.27.2", "strum_macros 0.27.2",
"sys-locale", "sys-locale",
"tempfile",
"tracing", "tracing",
"ts-rs", "ts-rs",
"uuid", "uuid",

View File

@@ -416,6 +416,7 @@ impl Session {
error!("failed to initialize rollout recorder: {e:#}"); error!("failed to initialize rollout recorder: {e:#}");
anyhow::anyhow!("failed to initialize rollout recorder: {e:#}") anyhow::anyhow!("failed to initialize rollout recorder: {e:#}")
})?; })?;
let rollout_path = rollout_recorder.rollout_path.clone();
// Create the mutable state for the Session. // Create the mutable state for the Session.
let state = State { let state = State {
history: ConversationHistory::new(), history: ConversationHistory::new(),
@@ -509,6 +510,7 @@ impl Session {
history_log_id, history_log_id,
history_entry_count, history_entry_count,
initial_messages, initial_messages,
rollout_path,
}), }),
}) })
.chain(post_session_configured_error_events.into_iter()); .chain(post_session_configured_error_events.into_iter());

View File

@@ -72,6 +72,7 @@ pub struct SavedSession {
#[derive(Clone)] #[derive(Clone)]
pub struct RolloutRecorder { pub struct RolloutRecorder {
tx: Sender<RolloutCmd>, tx: Sender<RolloutCmd>,
pub(crate) rollout_path: PathBuf,
} }
#[derive(Clone)] #[derive(Clone)]
@@ -119,13 +120,14 @@ impl RolloutRecorder {
/// cannot be created or the rollout file cannot be opened we return the /// cannot be created or the rollout file cannot be opened we return the
/// error so the caller can decide whether to disable persistence. /// error so the caller can decide whether to disable persistence.
pub async fn new(config: &Config, params: RolloutRecorderParams) -> std::io::Result<Self> { pub async fn new(config: &Config, params: RolloutRecorderParams) -> std::io::Result<Self> {
let (file, meta) = match params { let (file, rollout_path, meta) = match params {
RolloutRecorderParams::Create { RolloutRecorderParams::Create {
conversation_id, conversation_id,
instructions, instructions,
} => { } => {
let LogFileInfo { let LogFileInfo {
file, file,
path,
conversation_id: session_id, conversation_id: session_id,
timestamp, timestamp,
} = create_log_file(config, conversation_id)?; } = create_log_file(config, conversation_id)?;
@@ -140,6 +142,7 @@ impl RolloutRecorder {
( (
tokio::fs::File::from_std(file), tokio::fs::File::from_std(file),
path,
Some(SessionMeta { Some(SessionMeta {
timestamp, timestamp,
id: session_id, id: session_id,
@@ -150,8 +153,9 @@ impl RolloutRecorder {
RolloutRecorderParams::Resume { path } => ( RolloutRecorderParams::Resume { path } => (
tokio::fs::OpenOptions::new() tokio::fs::OpenOptions::new()
.append(true) .append(true)
.open(path) .open(&path)
.await?, .await?,
path,
None, None,
), ),
}; };
@@ -169,7 +173,7 @@ impl RolloutRecorder {
// driver instead of blocking the runtime. // driver instead of blocking the runtime.
tokio::task::spawn(rollout_writer(file, rx, meta, cwd)); tokio::task::spawn(rollout_writer(file, rx, meta, cwd));
Ok(Self { tx }) Ok(Self { tx, rollout_path })
} }
pub(crate) async fn record_items(&self, items: &[ResponseItem]) -> std::io::Result<()> { pub(crate) async fn record_items(&self, items: &[ResponseItem]) -> std::io::Result<()> {
@@ -289,6 +293,9 @@ struct LogFileInfo {
/// Opened file handle to the rollout file. /// Opened file handle to the rollout file.
file: File, file: File,
/// Full path to the rollout file.
path: PathBuf,
/// Session ID (also embedded in filename). /// Session ID (also embedded in filename).
conversation_id: ConversationId, conversation_id: ConversationId,
@@ -328,6 +335,7 @@ fn create_log_file(
Ok(LogFileInfo { Ok(LogFileInfo {
file, file,
path,
conversation_id, conversation_id,
timestamp, timestamp,
}) })

View File

@@ -523,6 +523,7 @@ impl EventProcessor for EventProcessorWithHumanOutput {
history_log_id: _, history_log_id: _,
history_entry_count: _, history_entry_count: _,
initial_messages: _, initial_messages: _,
rollout_path: _,
} = session_configured_event; } = session_configured_event;
ts_println!( ts_println!(

View File

@@ -528,6 +528,7 @@ impl CodexMessageProcessor {
let response = NewConversationResponse { let response = NewConversationResponse {
conversation_id, conversation_id,
model: session_configured.model, model: session_configured.model,
rollout_path: session_configured.rollout_path,
}; };
self.outgoing.send_response(request_id, response).await; self.outgoing.send_response(request_id, response).await;
} }

View File

@@ -262,6 +262,7 @@ mod tests {
use codex_protocol::mcp_protocol::LoginChatGptCompleteNotification; use codex_protocol::mcp_protocol::LoginChatGptCompleteNotification;
use pretty_assertions::assert_eq; use pretty_assertions::assert_eq;
use serde_json::json; use serde_json::json;
use tempfile::NamedTempFile;
use uuid::Uuid; use uuid::Uuid;
use super::*; use super::*;
@@ -272,6 +273,7 @@ mod tests {
let outgoing_message_sender = OutgoingMessageSender::new(outgoing_tx); let outgoing_message_sender = OutgoingMessageSender::new(outgoing_tx);
let conversation_id = ConversationId::new(); let conversation_id = ConversationId::new();
let rollout_file = NamedTempFile::new().unwrap();
let event = Event { let event = Event {
id: "1".to_string(), id: "1".to_string(),
msg: EventMsg::SessionConfigured(SessionConfiguredEvent { msg: EventMsg::SessionConfigured(SessionConfiguredEvent {
@@ -280,6 +282,7 @@ mod tests {
history_log_id: 1, history_log_id: 1,
history_entry_count: 1000, history_entry_count: 1000,
initial_messages: None, initial_messages: None,
rollout_path: rollout_file.path().to_path_buf(),
}), }),
}; };
@@ -305,12 +308,14 @@ mod tests {
let outgoing_message_sender = OutgoingMessageSender::new(outgoing_tx); let outgoing_message_sender = OutgoingMessageSender::new(outgoing_tx);
let conversation_id = ConversationId::new(); let conversation_id = ConversationId::new();
let rollout_file = NamedTempFile::new().unwrap();
let session_configured_event = SessionConfiguredEvent { let session_configured_event = SessionConfiguredEvent {
session_id: conversation_id, session_id: conversation_id,
model: "gpt-4o".to_string(), model: "gpt-4o".to_string(),
history_log_id: 1, history_log_id: 1,
history_entry_count: 1000, history_entry_count: 1000,
initial_messages: None, initial_messages: None,
rollout_path: rollout_file.path().to_path_buf(),
}; };
let event = Event { let event = Event {
id: "1".to_string(), id: "1".to_string(),
@@ -340,6 +345,7 @@ mod tests {
"history_log_id": session_configured_event.history_log_id, "history_log_id": session_configured_event.history_log_id,
"history_entry_count": session_configured_event.history_entry_count, "history_entry_count": session_configured_event.history_entry_count,
"type": "session_configured", "type": "session_configured",
"rollout_path": rollout_file.path().to_path_buf(),
} }
}); });
assert_eq!(params.unwrap(), expected_params); assert_eq!(params.unwrap(), expected_params);

View File

@@ -90,6 +90,7 @@ async fn test_codex_jsonrpc_conversation_flow() {
let NewConversationResponse { let NewConversationResponse {
conversation_id, conversation_id,
model, model,
rollout_path: _,
} = new_conv_resp; } = new_conv_resp;
assert_eq!(model, "mock-model"); assert_eq!(model, "mock-model");

View File

@@ -59,6 +59,7 @@ async fn test_conversation_create_and_send_message_ok() {
let NewConversationResponse { let NewConversationResponse {
conversation_id, conversation_id,
model, model,
rollout_path: _,
} = to_response::<NewConversationResponse>(new_conv_resp) } = to_response::<NewConversationResponse>(new_conv_resp)
.expect("deserialize newConversation response"); .expect("deserialize newConversation response");
assert_eq!(model, "o3"); assert_eq!(model, "o3");

View File

@@ -28,6 +28,7 @@ uuid = { version = "1", features = ["serde", "v4"] }
[dev-dependencies] [dev-dependencies]
pretty_assertions = "1.4.1" pretty_assertions = "1.4.1"
tempfile = "3"
[package.metadata.cargo-shear] [package.metadata.cargo-shear]
# Required because the not imported as strum_macros in non-nightly builds. # Required because the not imported as strum_macros in non-nightly builds.

View File

@@ -203,6 +203,7 @@ pub struct NewConversationParams {
pub struct NewConversationResponse { pub struct NewConversationResponse {
pub conversation_id: ConversationId, pub conversation_id: ConversationId,
pub model: String, pub model: String,
pub rollout_path: PathBuf,
} }
#[derive(Serialize, Deserialize, Debug, Clone, TS)] #[derive(Serialize, Deserialize, Debug, Clone, TS)]

View File

@@ -958,6 +958,8 @@ pub struct SessionConfiguredEvent {
/// When present, UIs can use these to seed the history. /// When present, UIs can use these to seed the history.
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
pub initial_messages: Option<Vec<EventMsg>>, pub initial_messages: Option<Vec<EventMsg>>,
pub rollout_path: PathBuf,
} }
/// User's decision in response to an ExecApprovalRequest. /// User's decision in response to an ExecApprovalRequest.
@@ -1020,12 +1022,15 @@ pub enum TurnAbortReason {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use serde_json::json;
use tempfile::NamedTempFile;
/// Serialize Event to verify that its JSON representation has the expected /// Serialize Event to verify that its JSON representation has the expected
/// amount of nesting. /// amount of nesting.
#[test] #[test]
fn serialize_event() { fn serialize_event() {
let conversation_id = ConversationId(uuid::uuid!("67e55044-10b1-426f-9247-bb680e5fe0c8")); let conversation_id = ConversationId(uuid::uuid!("67e55044-10b1-426f-9247-bb680e5fe0c8"));
let rollout_file = NamedTempFile::new().unwrap();
let event = Event { let event = Event {
id: "1234".to_string(), id: "1234".to_string(),
msg: EventMsg::SessionConfigured(SessionConfiguredEvent { msg: EventMsg::SessionConfigured(SessionConfiguredEvent {
@@ -1034,13 +1039,22 @@ mod tests {
history_log_id: 0, history_log_id: 0,
history_entry_count: 0, history_entry_count: 0,
initial_messages: None, initial_messages: None,
rollout_path: rollout_file.path().to_path_buf(),
}), }),
}; };
let serialized = serde_json::to_string(&event).unwrap();
assert_eq!( let expected = json!({
serialized, "id": "1234",
r#"{"id":"1234","msg":{"type":"session_configured","session_id":"67e55044-10b1-426f-9247-bb680e5fe0c8","model":"codex-mini-latest","history_log_id":0,"history_entry_count":0}}"# "msg": {
); "type": "session_configured",
"session_id": "67e55044-10b1-426f-9247-bb680e5fe0c8",
"model": "codex-mini-latest",
"history_log_id": 0,
"history_entry_count": 0,
"rollout_path": format!("{}", rollout_file.path().display()),
}
});
assert_eq!(expected, serde_json::to_value(&event).unwrap());
} }
#[test] #[test]

View File

@@ -35,6 +35,7 @@ use std::fs::File;
use std::io::BufRead; use std::io::BufRead;
use std::io::BufReader; use std::io::BufReader;
use std::path::PathBuf; use std::path::PathBuf;
use tempfile::NamedTempFile;
use tokio::sync::mpsc::unbounded_channel; use tokio::sync::mpsc::unbounded_channel;
fn test_config() -> Config { fn test_config() -> Config {
@@ -133,7 +134,7 @@ fn resumed_initial_messages_render_history() {
let (mut chat, mut rx, _ops) = make_chatwidget_manual(); let (mut chat, mut rx, _ops) = make_chatwidget_manual();
let conversation_id = ConversationId::new(); let conversation_id = ConversationId::new();
let rollout_file = NamedTempFile::new().unwrap();
let configured = codex_core::protocol::SessionConfiguredEvent { let configured = codex_core::protocol::SessionConfiguredEvent {
session_id: conversation_id, session_id: conversation_id,
model: "test-model".to_string(), model: "test-model".to_string(),
@@ -148,6 +149,7 @@ fn resumed_initial_messages_render_history() {
message: "assistant reply".to_string(), message: "assistant reply".to_string(),
}), }),
]), ]),
rollout_path: rollout_file.path().to_path_buf(),
}; };
chat.handle_codex_event(Event { chat.handle_codex_event(Event {

View File

@@ -605,6 +605,7 @@ pub(crate) fn new_session_info(
history_log_id: _, history_log_id: _,
history_entry_count: _, history_entry_count: _,
initial_messages: _, initial_messages: _,
rollout_path: _,
} = event; } = event;
if is_first_event { if is_first_event {
let cwd_str = match relativize_to_home(&config.cwd) { let cwd_str = match relativize_to_home(&config.cwd) {

View File

@@ -34,7 +34,7 @@
{"ts":"2025-08-09T15:51:04.829Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} {"ts":"2025-08-09T15:51:04.829Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"}
{"ts":"2025-08-09T15:51:04.829Z","dir":"to_tui","kind":"log_line","line":"[INFO codex_core::codex] resume_path: None"} {"ts":"2025-08-09T15:51:04.829Z","dir":"to_tui","kind":"log_line","line":"[INFO codex_core::codex] resume_path: None"}
{"ts":"2025-08-09T15:51:04.830Z","dir":"to_tui","kind":"app_event","variant":"Redraw"} {"ts":"2025-08-09T15:51:04.830Z","dir":"to_tui","kind":"app_event","variant":"Redraw"}
{"ts":"2025-08-09T15:51:04.856Z","dir":"to_tui","kind":"codex_event","payload":{"id":"0","msg":{"type":"session_configured","session_id":"d126e3d0-80ed-480a-be8c-09d97ff602cf","model":"gpt-5","history_log_id":2532619,"history_entry_count":339}}} {"ts":"2025-08-09T15:51:04.856Z","dir":"to_tui","kind":"codex_event","payload":{"id":"0","msg":{"type":"session_configured","session_id":"d126e3d0-80ed-480a-be8c-09d97ff602cf","model":"gpt-5","history_log_id":2532619,"history_entry_count":339,"rollout_path":"/tmp/codex-test-rollout.jsonl"}}}
{"ts":"2025-08-09T15:51:04.856Z","dir":"to_tui","kind":"insert_history","lines":9} {"ts":"2025-08-09T15:51:04.856Z","dir":"to_tui","kind":"insert_history","lines":9}
{"ts":"2025-08-09T15:51:04.857Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} {"ts":"2025-08-09T15:51:04.857Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"}
{"ts":"2025-08-09T15:51:04.857Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} {"ts":"2025-08-09T15:51:04.857Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"}
@@ -16447,7 +16447,7 @@
{"ts":"2025-08-09T16:06:58.083Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} {"ts":"2025-08-09T16:06:58.083Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"}
{"ts":"2025-08-09T16:06:58.085Z","dir":"to_tui","kind":"app_event","variant":"Redraw"} {"ts":"2025-08-09T16:06:58.085Z","dir":"to_tui","kind":"app_event","variant":"Redraw"}
{"ts":"2025-08-09T16:06:58.085Z","dir":"to_tui","kind":"log_line","line":"[INFO codex_core::codex] resume_path: None"} {"ts":"2025-08-09T16:06:58.085Z","dir":"to_tui","kind":"log_line","line":"[INFO codex_core::codex] resume_path: None"}
{"ts":"2025-08-09T16:06:58.136Z","dir":"to_tui","kind":"codex_event","payload":{"id":"0","msg":{"type":"session_configured","session_id":"c7df96da-daec-4fe9-aed9-3cd19b7a6192","model":"gpt-5","history_log_id":2532619,"history_entry_count":342}}} {"ts":"2025-08-09T16:06:58.136Z","dir":"to_tui","kind":"codex_event","payload":{"id":"0","msg":{"type":"session_configured","session_id":"c7df96da-daec-4fe9-aed9-3cd19b7a6192","model":"gpt-5","history_log_id":2532619,"history_entry_count":342,"rollout_path":"/tmp/codex-test-rollout.jsonl"}}}
{"ts":"2025-08-09T16:06:58.136Z","dir":"to_tui","kind":"insert_history","lines":9} {"ts":"2025-08-09T16:06:58.136Z","dir":"to_tui","kind":"insert_history","lines":9}
{"ts":"2025-08-09T16:06:58.136Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} {"ts":"2025-08-09T16:06:58.136Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"}
{"ts":"2025-08-09T16:06:58.136Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} {"ts":"2025-08-09T16:06:58.136Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"}