Repository navigation
[agent] Bound the SSE read after finish_reason so a stream without [DONE] cannot stall the call #176
Description
Found while delivering #174 / PR #175, which made this path read to data: [DONE]. Filed rather than
folded in, because #175 fixed the usage gap and this is a separate, unfixed one.
Context
PR #175 made the OpenAI-compatible SSE reader run to data: [DONE] instead of returning at
finish_reason, because the usage chunk that answers stream_options.include_usage arrives after
that marker. The trade that came with it is unbounded.
Measured at d5a625a, against an endpoint that streams a complete answer, sends finish_reason,
then holds the connection open (no [DONE], no close):
| Endpoint ends with | elapsed | Content | err |
|---|---|---|---|
data: [DONE] |
2 ms | "Hi" |
nil |
| nothing (connection held open) | 1.502 s | "Hi" |
nil |
The 1.502 s is the caller's context deadline in that run. The TUI and the one-shot CLI pass no
deadline of their own, so the real bound is agent/provider_openai.go:133 —
http.Client{Timeout: 120 * time.Second}, the whole request. For that entire window a run looks like
it is still streaming an answer that was already complete.
The delay is the smaller half. chatStream returns the partial result with nil error
(agent/provider_openai.go:314-316 only logs scanner.Err()), so the agent loop takes a truncated
stream for a finished answer: nothing is retried, nothing is reported, and the transcript gets
whatever text arrived before the cut.
The Ollama provider already has the mechanism this needs — ollamaIdleTimeout
(agent/provider_ollama.go:39), reset after every line (agent/provider_ollama.go:277), with a
cancel cause that names the bound that fired.
What to build
- An idle bound on the OpenAI-compatible stream read: fail when no line arrives for a named interval,
reset the interval on every line, and name the bound in the error. - Make the bound a package variable, so a test can shrink it (the Ollama pair are already test-shrunk).
- Keep the usage chunk reachable. A shortcut that stops at
finish_reasononce usage has been seen
is not a fix: an endpoint that never reports usage would return at the wrong point, and the flush
that follows the marker would be cut.
Acceptance
- A test server that streams deltas,
finish_reason, then holds the connection open returns within
the shrunk bound — not at the caller's deadline — and the error names the bound. - The same server, when it then sends a usage chunk and
[DONE], still returns the full content
and the reported usage: the bound must not cut a stream that is merely slow. - A slow-but-alive stream (a line every half-interval, several lines) is not cut.
go test ./... -count=1andgo test ./... -count=1 -racegreen.
Reproduction
Scratch module at /tmp/dsh-repro-175 (replace → this repository): cd /tmp/dsh-repro-175 && go run .
prints both rows of the table above.
Activity
yusiwen commentedon Oct 10, 2026
Landed as PR #178, rebase-merged to
masteras49964e2.What the fix is:
agent/provider_openai.gonow carries the same pair of bounds the Ollamaprovider has had since issue #1 —
openAIRequestTimeout(120 s) for one non-streaming request, andopenAIIdleTimeout(2 min) for the silence between lines of a stream, reset by every line, with thecancel cause naming the bound. A failed read returns
openai stream: …instead of the partialanswer with a nil error, and
openAIRequestErrorkeepserrors.Is(err, context.Canceled)for acaller cancellation so an interrupted run still reads as interrupted.
Measurements
Repro (
/tmp/dsh-repro-175), endpoint that completes its answer and then holds the connection open:data: [DONE]"Hi",err=nil"Hi",err=nil"Hi",err=nilresp=nil,err=openai stream: context deadline exceededThe unit test shrinks the bound to 400 ms and the silence case fails there — 0.40 s — instead of
waiting for anything longer.
go test ./... -count=1and-race: 13 packages ok, 0 failed, before and after the merge.CI: nine checks green, 11 annotations = the baseline, per job. No new annotations.
Main-module test functions: 767 → 769 (+2).
The mutation check changed the code. Removing the caller-cancellation classification still
passed the streaming subtest, because an HTTP/1.1 read error already wraps
context.Canceled. Ratherthan keep a guard that could not fail, the helper got its own unit test fed an HTTP/2-shaped error
(
http2: stream closed) — and the mutation now fails it. Four mutations, four failures; the"watchdog never fires" one fails as a 25 s test timeout, which is the read blocking without the
bound.
Left open deliberately: the bound is uniform, so a stream that goes quiet after a complete answer
still waits the full idle bound before failing; and a stream that closes without
[DONE]is stillaccepted as complete, because OpenAI-compatible servers vary and failing their working answers costs
more than the truncation it would catch.
fix(agent): bound the OpenAI-compatible stream after finish_reason
tests(agent): cover the OpenAI batch request bound
yusiwen commentedon Oct 11, 2026
Closing the trail on this one: the coverage half of the work also landed.
49964e2(#178), as recorded above.TestOpenAIBatchTimesOutOnSilentEndpoint, pinningopenAIRequestTimeouton the non-streaming path):a6b86c8(#177, authored by @AdamMagued).The "left open deliberately" residuals in the comment above are now tracked with measured repros:
http.Client.Timeoutcovers the streaming body read, so a working generation longer than 120 s is still cut.