Skip to content

Conversation

@owenlin0
Copy link
Contributor

@owenlin0 owenlin0 commented Nov 30, 2025

Emit item/fileChange/outputDelta instead of item/execCommand/outputDelta for FileChange items.

The underlying EventMsg::ExecCommandOutputDelta is emitted for apply_patch, shell, and unified exec, and since we represent apply_patch calls as FileChange items, we were seeing these events:

item/started <- FileChange
item/execCommand/outputDelta <- our code assumed that EventMsg::ExecCommandOutputDelta is always for the CommandExecution item
item/completed <- FileChange

With this fix we'll have these events:

item/started <- FileChange
item/fileChange/outputDelta <- our code assumed that EventMsg::ExecCommandOutputDelta is always for the CommandExecution item
item/completed <- FileChange

@owenlin0 owenlin0 force-pushed the owen/fix_apply_patch_delta branch from b3b69fe to c84a89d Compare November 30, 2025 18:21
@owenlin0 owenlin0 marked this pull request as ready for review December 1, 2025 16:03
@owenlin0 owenlin0 requested review from celia-oai and jif-oai December 1, 2025 16:05
let notification = CommandExecutionOutputDeltaNotification {
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();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment to explain the condition which is built here. I'm not sure to fully understand it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a comment!

@owenlin0 owenlin0 force-pushed the owen/fix_apply_patch_delta branch from c1be76e to 2d0f224 Compare December 1, 2025 16:52
@owenlin0 owenlin0 force-pushed the owen/fix_apply_patch_delta branch from 2d0f224 to 482980e Compare December 1, 2025 17:36
@owenlin0 owenlin0 enabled auto-merge (squash) December 1, 2025 17:48
@owenlin0 owenlin0 merged commit 8532876 into main Dec 1, 2025
26 checks passed
@owenlin0 owenlin0 deleted the owen/fix_apply_patch_delta branch December 1, 2025 17:52
@github-actions github-actions bot locked and limited conversation to collaborators Dec 1, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants