Skip to content

Fix CLI streaming invocation close race - #3804

Merged
vigoo merged 1 commit into
mainfrom
fix/cli-streaming-close-race
Sep 1, 2026
Merged

Fix CLI streaming invocation close race#3804
vigoo merged 1 commit into
mainfrom
fix/cli-streaming-close-race

Conversation

@vigoo

@vigoo vigoo commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Problem

A streaming agent can cancel its input while the CLI still has stdin frames queued in its independent WebSocket writer. The server then sends StreamCancel(InputConsumer), Finished(Success), and a normal WebSocket close.

If the CLI reader observes the close before the writer drains those queued frames, tungstenite moves the shared split socket into its closing state. The writer's next frame then returns ProtocolError::SendAfterClosing. The CLI did not classify that terminal local write state as a closed connection, so a successfully completed invocation could fail with a WebSocket protocol error. The same race could also mask a malformed piped-input diagnostic collected during cancellation.

Fix

  • Treat only tungstenite's SendAfterClosing protocol variant as a closed connection, alongside ConnectionClosed, AlreadyClosed, and broken pipe.
  • Add a deterministic in-memory split-WebSocket regression that reads a peer close and then attempts a write, reproducing the exact tungstenite state transition.
  • Verify that malformed peer traffic (ReceivedAfterClosing) remains an error.

This remains safe before protocol completion: a closed writer only stops outbound input handling, while the invocation still cannot succeed until the reader receives and validates Finished.

Verification

  • cargo test -p golem-cli --lib -- peer_close_racing_with_a_send_is_connection_closed connection_truncation_before_finish_is_an_error --report-time
  • integration::app::agents::test_streaming_invocation_cli_end_to_end passed 5 consecutive runs
  • cargo fmt -p golem-cli -- --check
  • cargo clippy -p golem-cli --all-targets --no-deps -- -D warnings
  • cargo check -p golem-cli --all-targets
  • cargo make build-cli-test-bins

@vigoo
vigoo requested a review from a team September 1, 2026 10:05
@netlify

netlify Bot commented Sep 1, 2026

Copy link
Copy Markdown

Deploy Preview for golemcloud canceled.

Name Link
🔨 Latest commit d430925
🔍 Latest deploy log https://app.netlify.com/projects/golemcloud/deploys/6a96a36f3c680e0008af481e

@vigoo
vigoo merged commit d12a46c into main Sep 1, 2026
103 of 104 checks passed
@vigoo
vigoo deleted the fix/cli-streaming-close-race branch September 1, 2026 11:00
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 1, 2026
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.

1 participant