From a3cccbd8ed7b92dbdf76f85210e17d569e6102ff Mon Sep 17 00:00:00 2001 From: Ruslan Nigmatullin Date: Fri, 24 Apr 2026 12:31:13 -0700 Subject: [PATCH] [codex] Omit fork turns from thread started notifications (#19093) ## Why `thread/fork` responses intentionally include copied history so the caller can render the fork immediately, but `thread/started` is a lifecycle notification. The v2 `Thread` contract says notifications should return `turns: []`, and the fork path was reusing the response thread directly, causing copied turns to be emitted through `thread/started` as well. ## What Changed - Route app-server `thread/started` notification construction through a helper that clears `thread.turns` before sending. - Keep `thread/fork` responses unchanged so callers still receive copied history. - Add persistent and ephemeral fork coverage that asserts `thread/started` emits an empty `turns` array while the response retains fork history. ## Testing - `just fmt` - `cargo test -p codex-app-server` --- .../app-server/src/codex_message_processor.rs | 11 ++++++++--- .../app-server/tests/suite/v2/thread_fork.rs | 18 ++++++++++++++++-- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 37d1bb87a..ecadef97f 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -2859,7 +2859,7 @@ impl CodexMessageProcessor { )) .await; - let notif = ThreadStartedNotification { thread }; + let notif = thread_started_notification(thread); listener_task_context .outgoing .send_server_notification(ServerNotification::ThreadStarted(notif)) @@ -5365,7 +5365,7 @@ impl CodexMessageProcessor { .await; } - let notif = ThreadStartedNotification { thread }; + let notif = thread_started_notification(thread); self.outgoing .send_server_notification(ServerNotification::ThreadStarted(notif)) .await; @@ -7744,7 +7744,7 @@ impl CodexMessageProcessor { .await, /*has_in_progress_turn*/ false, ); - let notif = ThreadStartedNotification { thread }; + let notif = thread_started_notification(thread); self.outgoing .send_server_notification(ServerNotification::ThreadStarted(notif)) .await; @@ -10094,6 +10094,11 @@ fn build_thread_from_snapshot( } } +fn thread_started_notification(mut thread: Thread) -> ThreadStartedNotification { + thread.turns.clear(); + ThreadStartedNotification { thread } +} + pub(crate) fn summary_to_thread( summary: ConversationSummary, fallback_cwd: &AbsolutePathBuf, diff --git a/codex-rs/app-server/tests/suite/v2/thread_fork.rs b/codex-rs/app-server/tests/suite/v2/thread_fork.rs index 7741ced16..7274daaa5 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_fork.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_fork.rs @@ -181,9 +181,16 @@ async fn thread_fork_creates_new_thread_and_emits_started() -> Result<()> { Some(&Value::Null), "thread/started must serialize `name: null` when unset" ); + assert_eq!( + started_thread_json.get("turns"), + Some(&json!([])), + "thread/started must not emit copied fork turns" + ); let started: ThreadStartedNotification = serde_json::from_value(notif.params.expect("params must be present"))?; - assert_eq!(started.thread, thread); + let mut expected_started_thread = thread; + expected_started_thread.turns.clear(); + assert_eq!(started.thread, expected_started_thread); Ok(()) } @@ -582,9 +589,16 @@ async fn thread_fork_ephemeral_remains_pathless_and_omits_listing() -> Result<() Some(true), "thread/started should serialize `ephemeral: true` for ephemeral forks" ); + assert_eq!( + started_thread_json.get("turns"), + Some(&json!([])), + "thread/started must not emit copied ephemeral fork turns" + ); let started: ThreadStartedNotification = serde_json::from_value(notif.params.expect("params must be present"))?; - assert_eq!(started.thread, thread); + let mut expected_started_thread = thread; + expected_started_thread.turns.clear(); + assert_eq!(started.thread, expected_started_thread); let list_id = mcp .send_thread_list_request(ThreadListParams {