Skip to content

Re-order shell call events#42

Merged
ScriptSmith merged 2 commits into
mainfrom
shell-event-ordering
May 31, 2026
Merged

Re-order shell call events#42
ScriptSmith merged 2 commits into
mainfrom
shell-event-ordering

Conversation

@ScriptSmith
Copy link
Copy Markdown
Owner

No description provided.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 31, 2026

Greptile Summary

This PR re-orders the terminal SSE events emitted by shell calls so that shell_call_output.done always arrives on the wire before its paired shell_call.done. The change applies uniformly to both the success and failure paths, with the success path extracted into a new emit_success_done helper that mirrors the existing emit_failure_done.

  • Ordering fix (both paths): swaps the two send calls so format_shell_call_output_item fires first; the ordering invariant is documented in the new helper's doc-comment and enforced by two new #[tokio::test] unit tests — one per path — that capture channel events and assert their order.
  • Refactor: the inline success-path emission previously written out long-hand inside ShellExecutor is replaced by a call to emit_success_done, keeping the two code paths structurally symmetric.

Confidence Score: 5/5

Safe to merge — the change is a pure ordering fix with no logic mutations, and the new ordering is guarded by two channel-capture tests covering both code paths.

Both the success and failure paths correctly swap to output-before-call ordering, the helper extraction is a clean refactor that preserves all parameters, and the new unit tests directly assert the wire ordering contract. No data is mutated and no new error surfaces are introduced.

No files require special attention.

Important Files Changed

Filename Overview
src/services/shell_tool.rs Re-orders output-before-call SSE event emission on both success and failure paths; extracts emit_success_done helper; adds two ordering tests. Logic, signatures, and parameter threading are all consistent with the existing format_shell_call_output_item/format_shell_call_item contracts.

Sequence Diagram

sequenceDiagram
    participant SE as ShellExecutor
    participant Ch as event_tx (ordered channel)
    participant Cl as SSE Consumer

    Note over SE,Cl: Success path (emit_success_done)
    SE->>Ch: format_shell_call_output_item(Done, exit_code, stdout, stderr, files, killed)
    SE->>Ch: "format_shell_call_item(Done, status=completed|incomplete)"
    Ch-->>Cl: shell_call_output.done  arrives first
    Ch-->>Cl: shell_call.done         arrives second

    Note over SE,Cl: Failure path (emit_failure_done)
    SE->>Ch: "format_shell_call_output_item(Done, -1, stderr, killed=true)"
    SE->>Ch: "format_shell_call_item(Done, status=incomplete)"
    Ch-->>Cl: shell_call_output.done  arrives first
    Ch-->>Cl: shell_call.done         arrives second
Loading

Reviews (3): Last reviewed commit: "Review fixes" | Re-trigger Greptile

Comment thread src/services/shell_tool.rs
@ScriptSmith
Copy link
Copy Markdown
Owner Author

@greptile-apps

@ScriptSmith ScriptSmith merged commit 824dd15 into main May 31, 2026
20 checks passed
@ScriptSmith ScriptSmith deleted the shell-event-ordering branch May 31, 2026 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant