Cancel AG-UI SSE request when the consumer stops early#506
Open
PratikDhanave wants to merge 4 commits into
Open
Cancel AG-UI SSE request when the consumer stops early#506PratikDhanave wants to merge 4 commits into
PratikDhanave wants to merge 4 commits into
Conversation
The aguiprovider streaming run passed the caller's context straight to the SSE client's Stream. On any early return from the run closure — the consumer stopping mid-stream, or a decode/event error — the request was never cancelled, so the client's background reader goroutine (which owns the HTTP response body) leaked until the client's ReadTimeout (5 minutes by default, or forever when ReadTimeout is 0). agent.Run does not supply a cancellable context, so nothing else tore the request down. Derive a cancellable context in the run closure and defer its cancel, so every early return releases the reader goroutine and response body. The sibling a2a and copilot providers already tear down on early return, so this brings aguiprovider in line. Adds TestAGUIAgentRun_EarlyStreamStop_CancelsUpstreamRequest: the server keeps the stream open after one event and waits for the client to disconnect; the test fails (times out on disconnect) before this change and passes after.
There was a problem hiding this comment.
Pull request overview
Fixes a resource-leak scenario in the AG-UI SSE streaming provider by ensuring upstream SSE requests are cancelled when the downstream consumer stops early (or when decode/event errors cause an early return), preventing the SSE client’s background reader goroutine and HTTP response body from lingering until timeout.
Changes:
- Wrap the provider’s streaming request context with
context.WithCancelanddefer cancel()to guarantee teardown on any early return. - Add a regression test that verifies the server observes a client disconnect promptly when the consumer breaks out of the stream early.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| provider/aguiprovider/agui.go | Ensures SSE request cancellation on early stream termination to avoid leaking the client reader goroutine/response body. |
| provider/aguiprovider/agui_leak_test.go | Adds a targeted regression test confirming early consumer stop triggers upstream disconnect/cancellation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
aguiproviderstreaming run passed the caller'scontextstraight to the SSE client'sStream. On any early return from the run closure — the consumer stopping mid-stream (yieldreturns false), or a decode/event error — the request was never cancelled, so the client's background reader goroutine (which owns the HTTP response body) leaked until the client'sReadTimeout(5 minutes by default, or indefinitely whenReadTimeoutis 0).agent.Runwraps the context only withcontext.WithValue, so nothing else tore the request down.Fix
Derive a cancellable context in the run closure and
defer cancel(), so every early return releases the reader goroutine and response body:The sibling
a2a(pull-based iterator withdefer body.Close()) andcopilot(deferredunsubscribe/Disconnect) providers already tear down on early return; this bringsaguiproviderin line.Public API
No exported symbols change.
Tests
Adds
TestAGUIAgentRun_EarlyStreamStop_CancelsUpstreamRequest: the test server sends one event, keeps the stream open, and waits for the client to disconnect. It fails before this change (the disconnect never arrives; the handler's safety timeout expires) and passes after (early stop cancels the request, server observes the disconnect promptly).go test -race ./provider/aguiproviderpasses;go build ./...andgo vetclean; gofumpt clean.