[app-server] fix: emit item/fileChange/outputDelta for file change items (#7399)
This commit is contained in:
parent
44d92675eb
commit
8532876ad8
4 changed files with 70 additions and 10 deletions
|
|
@ -511,6 +511,7 @@ server_notification_definitions! {
|
|||
ItemCompleted => "item/completed" (v2::ItemCompletedNotification),
|
||||
AgentMessageDelta => "item/agentMessage/delta" (v2::AgentMessageDeltaNotification),
|
||||
CommandExecutionOutputDelta => "item/commandExecution/outputDelta" (v2::CommandExecutionOutputDeltaNotification),
|
||||
FileChangeOutputDelta => "item/fileChange/outputDelta" (v2::FileChangeOutputDeltaNotification),
|
||||
McpToolCallProgress => "item/mcpToolCall/progress" (v2::McpToolCallProgressNotification),
|
||||
AccountUpdated => "account/updated" (v2::AccountUpdatedNotification),
|
||||
AccountRateLimitsUpdated => "account/rateLimits/updated" (v2::AccountRateLimitsUpdatedNotification),
|
||||
|
|
|
|||
|
|
@ -1353,6 +1353,16 @@ pub struct CommandExecutionOutputDeltaNotification {
|
|||
pub delta: String,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct FileChangeOutputDeltaNotification {
|
||||
pub thread_id: String,
|
||||
pub turn_id: String,
|
||||
pub item_id: String,
|
||||
pub delta: String,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ use codex_app_server_protocol::ContextCompactedNotification;
|
|||
use codex_app_server_protocol::ErrorNotification;
|
||||
use codex_app_server_protocol::ExecCommandApprovalParams;
|
||||
use codex_app_server_protocol::ExecCommandApprovalResponse;
|
||||
use codex_app_server_protocol::FileChangeOutputDeltaNotification;
|
||||
use codex_app_server_protocol::FileChangeRequestApprovalParams;
|
||||
use codex_app_server_protocol::FileChangeRequestApprovalResponse;
|
||||
use codex_app_server_protocol::FileUpdateChange;
|
||||
|
|
@ -501,17 +502,44 @@ pub(crate) async fn apply_bespoke_event_handling(
|
|||
.await;
|
||||
}
|
||||
EventMsg::ExecCommandOutputDelta(exec_command_output_delta_event) => {
|
||||
let notification = CommandExecutionOutputDeltaNotification {
|
||||
thread_id: conversation_id.to_string(),
|
||||
turn_id: event_turn_id.clone(),
|
||||
item_id: exec_command_output_delta_event.call_id.clone(),
|
||||
delta: String::from_utf8_lossy(&exec_command_output_delta_event.chunk).to_string(),
|
||||
let item_id = exec_command_output_delta_event.call_id.clone();
|
||||
let delta = String::from_utf8_lossy(&exec_command_output_delta_event.chunk).to_string();
|
||||
// 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 map = turn_summary_store.lock().await;
|
||||
map.get(&conversation_id)
|
||||
.is_some_and(|summary| summary.file_change_started.contains(&item_id))
|
||||
};
|
||||
outgoing
|
||||
.send_server_notification(ServerNotification::CommandExecutionOutputDelta(
|
||||
notification,
|
||||
))
|
||||
.await;
|
||||
if is_file_change {
|
||||
let notification = FileChangeOutputDeltaNotification {
|
||||
thread_id: conversation_id.to_string(),
|
||||
turn_id: event_turn_id.clone(),
|
||||
item_id,
|
||||
delta,
|
||||
};
|
||||
outgoing
|
||||
.send_server_notification(ServerNotification::FileChangeOutputDelta(
|
||||
notification,
|
||||
))
|
||||
.await;
|
||||
} else {
|
||||
let notification = CommandExecutionOutputDeltaNotification {
|
||||
thread_id: conversation_id.to_string(),
|
||||
turn_id: event_turn_id.clone(),
|
||||
item_id,
|
||||
delta,
|
||||
};
|
||||
outgoing
|
||||
.send_server_notification(ServerNotification::CommandExecutionOutputDelta(
|
||||
notification,
|
||||
))
|
||||
.await;
|
||||
}
|
||||
}
|
||||
EventMsg::ExecCommandEnd(exec_command_end_event) => {
|
||||
let ExecCommandEndEvent {
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ use app_test_support::to_response;
|
|||
use codex_app_server_protocol::ApprovalDecision;
|
||||
use codex_app_server_protocol::CommandExecutionRequestApprovalResponse;
|
||||
use codex_app_server_protocol::CommandExecutionStatus;
|
||||
use codex_app_server_protocol::FileChangeOutputDeltaNotification;
|
||||
use codex_app_server_protocol::FileChangeRequestApprovalResponse;
|
||||
use codex_app_server_protocol::ItemCompletedNotification;
|
||||
use codex_app_server_protocol::ItemStartedNotification;
|
||||
|
|
@ -725,6 +726,26 @@ async fn turn_start_file_change_approval_v2() -> Result<()> {
|
|||
)
|
||||
.await?;
|
||||
|
||||
let output_delta_notif = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("item/fileChange/outputDelta"),
|
||||
)
|
||||
.await??;
|
||||
let output_delta: FileChangeOutputDeltaNotification = serde_json::from_value(
|
||||
output_delta_notif
|
||||
.params
|
||||
.clone()
|
||||
.expect("item/fileChange/outputDelta params"),
|
||||
)?;
|
||||
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 = timeout(DEFAULT_READ_TIMEOUT, async {
|
||||
loop {
|
||||
let completed_notif = mcp
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue