agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

Stop emitting item/fileChange/outputDelta output delta notifications (#20471)

Browse Source 
## 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)*
This commit is contained in:
pakrym-oai
2026-04-30 11:42:07 -07:00
committed by GitHub
Unverified
parent f2bc2f26a9
commit b52083146c
13 changed files with 59 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/README.md
+1 -1
View File
@@ -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
codex-rs/app-server/src/bespoke_event_handling.rs
-18
View File
@@ -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;
}
codex-rs/app-server/tests/suite/v2/turn_start.rs
+1 -33
View File
@@ -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<FileChangeOutputDeltaNotification> = None;
let mut completed_file_change: Option<ThreadItem> = 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"),