From b52083146c2ba05d129e144ac5a8d9d77e8dfb9d Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Thu, 30 Apr 2026 11:42:07 -0700 Subject: [PATCH] Stop emitting item/fileChange/outputDelta output delta notifications (#20471) ## Why `item/fileChange/outputDelta` text output was only the tool's summary or error text and not used by client surfaces. We keep `item/fileChange/outputDelta` in the app-server protocol as a deprecated compatibility entry, but the server no longer emits it. ## What changed - stop the `apply_patch` runtime from emitting `ExecCommandOutputDelta` events - simplify `item_event_to_server_notification` so command output deltas always map to `item/commandExecution/outputDelta` - remove the app-server bookkeeping that tried to detect whether an output delta belonged to a file change - mark `item/fileChange/outputDelta` as a deprecated legacy protocol entry in the v2 types, schema, and README - simplify the file-change approval tests so they only wait for completion instead of expecting output-delta notifications ## Testing - `cargo test -p codex-app-server-protocol` - `cargo test -p codex-thread-manager-sample` - `cargo test -p codex-app-server-protocol protocol::event_mapping::tests::exec_command_output_delta_maps_to_command_execution_output_delta -- --exact` - `cargo test -p codex-app-server turn_start_file_change_approval_accept_for_session_persists_v2 -- --exact` *(failed before the test assertions because the wiremock `/responses` mock received 0 requests in setup)* --- .../schema/json/ServerNotification.json | 2 + .../codex_app_server_protocol.schemas.json | 2 + .../codex_app_server_protocol.v2.schemas.json | 2 + .../v2/FileChangeOutputDeltaNotification.json | 1 + .../v2/FileChangeOutputDeltaNotification.ts | 5 ++ .../src/protocol/common.rs | 1 + .../src/protocol/event_mapping.rs | 62 ++++++++++++------- .../app-server-protocol/src/protocol/v2.rs | 3 + codex-rs/app-server/README.md | 2 +- .../app-server/src/bespoke_event_handling.rs | 18 ------ .../app-server/tests/suite/v2/turn_start.rs | 34 +--------- .../core/src/tools/runtimes/apply_patch.rs | 22 ------- codex-rs/thread-manager-sample/src/main.rs | 1 - 13 files changed, 59 insertions(+), 96 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index 6dbc66cb4..82914f3a6 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -1032,6 +1032,7 @@ "type": "object" }, "FileChangeOutputDeltaNotification": { + "description": "Deprecated legacy notification for `apply_patch` textual output.\n\nThe server no longer emits this notification.", "properties": { "delta": { "type": "string" @@ -5191,6 +5192,7 @@ "type": "object" }, { + "description": "Deprecated legacy apply_patch output stream notification.", "properties": { "method": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 9adc20bd3..b0687e708 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -4289,6 +4289,7 @@ "type": "object" }, { + "description": "Deprecated legacy apply_patch output stream notification.", "properties": { "method": { "enum": [ @@ -8600,6 +8601,7 @@ }, "FileChangeOutputDeltaNotification": { "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Deprecated legacy notification for `apply_patch` textual output.\n\nThe server no longer emits this notification.", "properties": { "delta": { "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index d581e19b9..c00abca26 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -5099,6 +5099,7 @@ }, "FileChangeOutputDeltaNotification": { "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Deprecated legacy notification for `apply_patch` textual output.\n\nThe server no longer emits this notification.", "properties": { "delta": { "type": "string" @@ -11307,6 +11308,7 @@ "type": "object" }, { + "description": "Deprecated legacy apply_patch output stream notification.", "properties": { "method": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/FileChangeOutputDeltaNotification.json b/codex-rs/app-server-protocol/schema/json/v2/FileChangeOutputDeltaNotification.json index 2b3abd67f..97d617ea4 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/FileChangeOutputDeltaNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/FileChangeOutputDeltaNotification.json @@ -1,5 +1,6 @@ { "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Deprecated legacy notification for `apply_patch` textual output.\n\nThe server no longer emits this notification.", "properties": { "delta": { "type": "string" diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/FileChangeOutputDeltaNotification.ts b/codex-rs/app-server-protocol/schema/typescript/v2/FileChangeOutputDeltaNotification.ts index 1018bd8a2..c11f626cd 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/FileChangeOutputDeltaNotification.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/FileChangeOutputDeltaNotification.ts @@ -2,4 +2,9 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +/** + * Deprecated legacy notification for `apply_patch` textual output. + * + * The server no longer emits this notification. + */ export type FileChangeOutputDeltaNotification = { threadId: string, turnId: string, itemId: string, delta: string, }; diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index f86b33eaa..e2865aa31 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -1397,6 +1397,7 @@ server_notification_definitions! { CommandExecOutputDelta => "command/exec/outputDelta" (v2::CommandExecOutputDeltaNotification), CommandExecutionOutputDelta => "item/commandExecution/outputDelta" (v2::CommandExecutionOutputDeltaNotification), TerminalInteraction => "item/commandExecution/terminalInteraction" (v2::TerminalInteractionNotification), + /// Deprecated legacy apply_patch output stream notification. FileChangeOutputDelta => "item/fileChange/outputDelta" (v2::FileChangeOutputDeltaNotification), FileChangePatchUpdated => "item/fileChange/patchUpdated" (v2::FileChangePatchUpdatedNotification), ServerRequestResolved => "serverRequest/resolved" (v2::ServerRequestResolvedNotification), diff --git a/codex-rs/app-server-protocol/src/protocol/event_mapping.rs b/codex-rs/app-server-protocol/src/protocol/event_mapping.rs index d86d45f16..f516fc528 100644 --- a/codex-rs/app-server-protocol/src/protocol/event_mapping.rs +++ b/codex-rs/app-server-protocol/src/protocol/event_mapping.rs @@ -10,7 +10,6 @@ use crate::protocol::v2::CollabAgentToolCallStatus; use crate::protocol::v2::CommandExecutionOutputDeltaNotification; use crate::protocol::v2::DynamicToolCallOutputContentItem; use crate::protocol::v2::DynamicToolCallStatus; -use crate::protocol::v2::FileChangeOutputDeltaNotification; use crate::protocol::v2::FileChangePatchUpdatedNotification; use crate::protocol::v2::ItemCompletedNotification; use crate::protocol::v2::ItemStartedNotification; @@ -37,7 +36,6 @@ pub fn item_event_to_server_notification( msg: EventMsg, thread_id: &str, turn_id: &str, - is_file_change_output: bool, ) -> ServerNotification { let thread_id = thread_id.to_string(); let turn_id = turn_id.to_string(); @@ -477,23 +475,14 @@ pub fn item_event_to_server_notification( EventMsg::ExecCommandOutputDelta(exec_command_output_delta_event) => { let item_id = exec_command_output_delta_event.call_id; let delta = String::from_utf8_lossy(&exec_command_output_delta_event.chunk).to_string(); - if is_file_change_output { - ServerNotification::FileChangeOutputDelta(FileChangeOutputDeltaNotification { + ServerNotification::CommandExecutionOutputDelta( + CommandExecutionOutputDeltaNotification { thread_id, turn_id, item_id, delta, - }) - } else { - ServerNotification::CommandExecutionOutputDelta( - CommandExecutionOutputDeltaNotification { - thread_id, - turn_id, - item_id, - delta, - }, - ) - } + }, + ) } EventMsg::TerminalInteraction(terminal_event) => { ServerNotification::TerminalInteraction(TerminalInteractionNotification { @@ -522,6 +511,8 @@ mod tests { use codex_protocol::mcp::CallToolResult; use codex_protocol::protocol::CollabResumeBeginEvent; use codex_protocol::protocol::CollabResumeEndEvent; + use codex_protocol::protocol::ExecCommandOutputDeltaEvent; + use codex_protocol::protocol::ExecOutputStream; use codex_protocol::protocol::McpInvocation; use codex_protocol::protocol::McpToolCallBeginEvent; use codex_protocol::protocol::McpToolCallEndEvent; @@ -549,6 +540,18 @@ mod tests { } } + fn assert_command_execution_output_delta_server_notification( + notification: ServerNotification, + expected: CommandExecutionOutputDeltaNotification, + ) { + match notification { + ServerNotification::CommandExecutionOutputDelta(payload) => { + assert_eq!(payload, expected) + } + other => panic!("expected command execution output delta, got {other:?}"), + } + } + #[test] fn collab_resume_begin_maps_to_item_started_resume_agent() { let event = CollabResumeBeginEvent { @@ -563,7 +566,6 @@ mod tests { EventMsg::CollabResumeBegin(event.clone()), "thread-1", "turn-1", - /*is_file_change_output*/ false, ); assert_item_started_server_notification( notification, @@ -601,7 +603,6 @@ mod tests { EventMsg::CollabResumeEnd(event.clone()), "thread-2", "turn-2", - /*is_file_change_output*/ false, ); assert_item_completed_server_notification( notification, @@ -644,7 +645,6 @@ mod tests { EventMsg::McpToolCallBegin(begin_event.clone()), "thread-1", "turn_1", - /*is_file_change_output*/ false, ); assert_item_started_server_notification( notification, @@ -682,7 +682,6 @@ mod tests { EventMsg::McpToolCallBegin(begin_event.clone()), "thread-2", "turn_2", - /*is_file_change_output*/ false, ); assert_item_started_server_notification( notification, @@ -735,7 +734,6 @@ mod tests { EventMsg::McpToolCallEnd(end_event.clone()), "thread-3", "turn_3", - /*is_file_change_output*/ false, ); assert_item_completed_server_notification( notification, @@ -781,7 +779,6 @@ mod tests { EventMsg::McpToolCallEnd(end_event.clone()), "thread-4", "turn_4", - /*is_file_change_output*/ false, ); assert_item_completed_server_notification( notification, @@ -804,4 +801,27 @@ mod tests { }, ); } + + #[test] + fn exec_command_output_delta_maps_to_command_execution_output_delta() { + let notification = item_event_to_server_notification( + EventMsg::ExecCommandOutputDelta(ExecCommandOutputDeltaEvent { + call_id: "call-1".to_string(), + stream: ExecOutputStream::Stdout, + chunk: b"hello".to_vec(), + }), + "thread-1", + "turn-1", + ); + + assert_command_execution_output_delta_server_notification( + notification, + CommandExecutionOutputDeltaNotification { + thread_id: "thread-1".to_string(), + turn_id: "turn-1".to_string(), + item_id: "call-1".to_string(), + delta: "hello".to_string(), + }, + ); + } } diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index f66078d4b..068a9c5f5 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -6992,6 +6992,9 @@ pub struct CommandExecOutputDeltaNotification { pub cap_reached: bool, } +/// Deprecated legacy notification for `apply_patch` textual output. +/// +/// The server no longer emits this notification. #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 4970f04ff..c001cef99 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1183,7 +1183,7 @@ There are additional item-specific events: #### fileChange - `item/fileChange/patchUpdated` - when `features.apply_patch_streaming_events` is enabled, streams structured file-change snapshots parsed from the model-generated patch before it is executed. -- `item/fileChange/outputDelta` - contains the tool call response of the underlying `apply_patch` tool call. +- `item/fileChange/outputDelta` - deprecated legacy protocol entry for `apply_patch` text output; retained for compatibility but no longer emitted by the server. ### Errors diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 00e13245b..e8a6cb9cc 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -887,7 +887,6 @@ pub(crate) async fn apply_bespoke_event_handling( msg, &conversation_id.to_string(), &event_turn_id, - /*is_file_change_output*/ false, ); outgoing.send_server_notification(notification).await; } @@ -905,7 +904,6 @@ pub(crate) async fn apply_bespoke_event_handling( EventMsg::CollabCloseEnd(end_event), &conversation_id.to_string(), &event_turn_id, - /*is_file_change_output*/ false, ); outgoing.send_server_notification(notification).await; } @@ -1041,7 +1039,6 @@ pub(crate) async fn apply_bespoke_event_handling( msg, &conversation_id.to_string(), &event_turn_id, - /*is_file_change_output*/ false, ); outgoing.send_server_notification(notification).await; } @@ -1124,7 +1121,6 @@ pub(crate) async fn apply_bespoke_event_handling( EventMsg::PatchApplyBegin(patch_begin_event), &conversation_id.to_string(), &event_turn_id, - /*is_file_change_output*/ false, ); outgoing.send_server_notification(notification).await; } @@ -1166,28 +1162,15 @@ pub(crate) async fn apply_bespoke_event_handling( EventMsg::ExecCommandBegin(exec_command_begin_event), &conversation_id.to_string(), &event_turn_id, - /*is_file_change_output*/ false, ); outgoing.send_server_notification(notification).await; } } EventMsg::ExecCommandOutputDelta(exec_command_output_delta_event) => { - let item_id = exec_command_output_delta_event.call_id.clone(); - // The underlying EventMsg::ExecCommandOutputDelta is used for shell, unified_exec, - // and apply_patch tool calls. We represent apply_patch with the FileChange item, and - // everything else with the CommandExecution item. - // - // We need to detect which item type it is so we can emit the right notification. - // We already have state tracking FileChange items on item/started, so let's use that. - let is_file_change = { - let state = thread_state.lock().await; - state.turn_summary.file_change_started.contains(&item_id) - }; let notification = item_event_to_server_notification( EventMsg::ExecCommandOutputDelta(exec_command_output_delta_event), &conversation_id.to_string(), &event_turn_id, - is_file_change, ); outgoing.send_server_notification(notification).await; } @@ -1213,7 +1196,6 @@ pub(crate) async fn apply_bespoke_event_handling( EventMsg::ExecCommandEnd(exec_command_end_event), &conversation_id.to_string(), &event_turn_id, - /*is_file_change_output*/ false, ); outgoing.send_server_notification(notification).await; } 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 aa507f39e..3c5bbd3b6 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -24,7 +24,6 @@ use codex_app_server_protocol::CommandExecutionApprovalDecision; use codex_app_server_protocol::CommandExecutionRequestApprovalResponse; use codex_app_server_protocol::CommandExecutionStatus; use codex_app_server_protocol::FileChangeApprovalDecision; -use codex_app_server_protocol::FileChangeOutputDeltaNotification; use codex_app_server_protocol::FileChangePatchUpdatedNotification; use codex_app_server_protocol::FileChangeRequestApprovalResponse; use codex_app_server_protocol::ItemCompletedNotification; @@ -2198,9 +2197,8 @@ async fn turn_start_file_change_approval_v2() -> Result<()> { ) .await?; let mut saw_resolved = false; - let mut output_delta: Option = None; let mut completed_file_change: Option = None; - while !(output_delta.is_some() && completed_file_change.is_some()) { + while completed_file_change.is_none() { let message = timeout(DEFAULT_READ_TIMEOUT, mcp.read_next_message()).await??; let JSONRPCMessage::Notification(notification) = message else { continue; @@ -2217,16 +2215,6 @@ async fn turn_start_file_change_approval_v2() -> Result<()> { assert_eq!(resolved.request_id, resolved_request_id); saw_resolved = true; } - "item/fileChange/outputDelta" => { - assert!(saw_resolved, "serverRequest/resolved should arrive first"); - let notification: FileChangeOutputDeltaNotification = serde_json::from_value( - notification - .params - .clone() - .expect("item/fileChange/outputDelta params"), - )?; - output_delta = Some(notification); - } "item/completed" => { let completed: ItemCompletedNotification = serde_json::from_value( notification.params.clone().expect("item/completed params"), @@ -2239,16 +2227,6 @@ async fn turn_start_file_change_approval_v2() -> Result<()> { _ => {} } } - let output_delta = output_delta.expect("file change output delta should be observed"); - assert_eq!(output_delta.thread_id, thread.id); - assert_eq!(output_delta.turn_id, turn.id); - assert_eq!(output_delta.item_id, "patch-call"); - assert!( - !output_delta.delta.is_empty(), - "expected delta to be non-empty, got: {}", - output_delta.delta - ); - let completed_file_change = completed_file_change.expect("file change completion should be observed"); let ThreadItem::FileChange { ref id, status, .. } = completed_file_change else { @@ -3000,11 +2978,6 @@ async fn turn_start_file_change_approval_accept_for_session_persists_v2() -> Res ) .await?; - timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_notification_message("item/fileChange/outputDelta"), - ) - .await??; timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_notification_message("item/completed"), @@ -3058,11 +3031,6 @@ async fn turn_start_file_change_approval_accept_for_session_persists_v2() -> Res // If the server incorrectly emits FileChangeRequestApproval, the helper below will error // (it bails on unexpected JSONRPCMessage::Request), causing the test to fail. - timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_notification_message("item/fileChange/outputDelta"), - ) - .await??; timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_notification_message("item/completed"), diff --git a/codex-rs/core/src/tools/runtimes/apply_patch.rs b/codex-rs/core/src/tools/runtimes/apply_patch.rs index 7f218a629..a25a06aac 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch.rs @@ -25,10 +25,6 @@ use codex_protocol::exec_output::ExecToolCallOutput; use codex_protocol::exec_output::StreamOutput; use codex_protocol::models::AdditionalPermissionProfile; use codex_protocol::protocol::AskForApproval; -use codex_protocol::protocol::Event; -use codex_protocol::protocol::EventMsg; -use codex_protocol::protocol::ExecCommandOutputDeltaEvent; -use codex_protocol::protocol::ExecOutputStream; use codex_protocol::protocol::FileChange; use codex_protocol::protocol::ReviewDecision; use codex_sandboxing::SandboxType; @@ -87,22 +83,6 @@ impl ApplyPatchRuntime { use_legacy_landlock: attempt.use_legacy_landlock, }) } - - async fn emit_output_delta(ctx: &ToolCtx, stream: ExecOutputStream, chunk: &[u8]) { - if chunk.is_empty() { - return; - } - - let event = Event { - id: ctx.turn.sub_id.clone(), - msg: EventMsg::ExecCommandOutputDelta(ExecCommandOutputDeltaEvent { - call_id: ctx.call_id.clone(), - stream, - chunk: chunk.to_vec(), - }), - }; - let _ = ctx.session.get_tx_event().send(event).await; - } } impl Sandboxable for ApplyPatchRuntime { @@ -230,8 +210,6 @@ impl ToolRuntime for ApplyPatchRuntime { .await; let stdout = String::from_utf8_lossy(&stdout).into_owned(); let stderr = String::from_utf8_lossy(&stderr).into_owned(); - Self::emit_output_delta(ctx, ExecOutputStream::Stdout, stdout.as_bytes()).await; - Self::emit_output_delta(ctx, ExecOutputStream::Stderr, stderr.as_bytes()).await; let exit_code = if result.is_ok() { 0 } else { 1 }; let output = ExecToolCallOutput { exit_code, diff --git a/codex-rs/thread-manager-sample/src/main.rs b/codex-rs/thread-manager-sample/src/main.rs index cf022fed1..b0cbf3243 100644 --- a/codex-rs/thread-manager-sample/src/main.rs +++ b/codex-rs/thread-manager-sample/src/main.rs @@ -320,7 +320,6 @@ async fn run_turn(thread: &CodexThread, thread_id: &str, prompt: String) -> anyh current_turn_id .as_deref() .context("mapped notification arrived before turn started")?, - /*is_file_change_output*/ false, )), _ => None, };