fix(runtime): deliver StreamStopped with a bounded blocking send - #4153
Merged
Conversation
finalizeEventChannel emitted the terminal StreamStoppedEvent non-blockingly and dropped it whenever the runtime's 128-slot event buffer was full at teardown. Under normal streaming back-pressure (a consumer, e.g. the PersistenceObserver, doing a synchronous per-delta SQLite write) the buffer is routinely full at that exact moment, so the TUI/supervisor/leantui never saw the event and stayed stuck on "Working…" until Esc (#4136). Replace the non-blocking emit with a bounded blocking send (~5s deadline, event_sink.go's new boundedChannelSink/bounded()): a consumer still draining the channel reliably receives it, even past context cancellation (the TUI keeps draining after Esc); it is dropped only once nothing accepts it within the deadline, i.e. the consumer has genuinely abandoned the channel. Deliberately not unbounded: a consumer audit found several call sites that legitimately stop reading before close (a2a, acp, cli runner, LocalRuntime.Run, embeddedchat, server RunSession), and an unbounded send there would reintroduce the #3070 teardown deadlock that #3074/#3275 previously fixed by making the emit non-blocking in the first place. Also harden pkg/a2a/adapter.go: it only completed the ADK turn from the StreamStoppedEvent case, so an ever-dropped event left the turn hanging. Add a channel-close fallback so a closed events channel without a prior StreamStopped still completes the turn.
aheritier
marked this pull request as ready for review
September 3, 2026 18:18
This was referenced Sep 3, 2026
dgageot
approved these changes
Sep 4, 2026
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
Fixes #4136: the TUI got stuck on "Working…" after the stream had actually finished, because
finalizeEventChannelemitted the terminalStreamStoppedEventnon-blockingly and silently dropped it whenever the runtime's 128-slot event buffer was full at teardown. Under normal streaming back-pressure (e.g.PersistenceObserverdoing a synchronous per-delta SQLite write, or any consumer slower than the model's delta rate) the buffer is routinely full at exactly that moment, so the TUI/supervisor/leantui — which only clear their busy state on receipt of that event — never learned the stream had ended.Change
finalizeEventChannelnow deliversStreamStoppedwith a bounded blocking send (~5s deadline, newboundedChannelSink/bounded()helper inevent_sink.go, mirroring the existingnonBlocking()). A consumer still draining the channel — even one that already sawctxcancelled (Esc) but keeps draining to close — reliably receives it. It's dropped only if nothing accepts it within the deadline, i.e. the consumer has genuinely abandoned the channel.LocalRuntime.Run, embeddedchat, serverRunSession). An unbounded send there would reintroduce the exact teardown deadlock fix: guard elicitation channel close against in-flight sends #3070 fixed, which decide StreamStopped ordering vs session-end hooks and document teardown trade-offs from #3070 #3074/docs(runtime): document StreamStopped ordering and teardown trade-offs (#3074) #3275 addressed by making this emit non-blocking (and documented delivery as best-effort) in the first place.finalizeEventChannel/StreamStoppedEventdoc comments to describe the new bounded-delivery contract; channel close remains the one guaranteed terminal signal.pkg/a2a/adapter.go: the ADK turn-complete event was only ever sent from theStreamStoppedEventcase, so a dropped event left the turn hanging. Added a channel-close fallback so it still completes if that event was never delivered.Tests
pkg/runtime/stream_stopped_delivery_test.go: a deterministic regression test reproducing the TUI shows "working" even after stream has ended for Qwen 3.8 (served locally via vLLM) #4136 race (slowEventObserver+ ≥250 streamed deltas) asserting exactly oneStreamStopped, last before close; plus a "slow-but-alive consumer eventually receives it" case. Passes-count=20with zero flakes.TestLocalRuntime_FinalizeEventChannelDoesNotDeadlockWhenBufferFullAndConsumerGone→TestLocalRuntime_FinalizeEventChannelDropsStreamStoppedAfterBoundedTimeout: injects a short timeout, asserts the send waits out the deadline (proving bounded, not instant-drop) and still returns/drops for a genuinely abandoned consumer.TestLocalRuntime_FinalizeEventChannelStreamStoppedIsLastBeforeClose/...EmitsStreamStoppedOncepass unchanged.go build,go test ./...,golangci-lint run(v2.13.1) all clean. (pkg/rag/treesitterfails in this sandbox only — pre-existing, missing gcc for CGO, unrelated to this change.)Independent review (sub-agent) returned only low-severity, non-blocking nits (comment verbosity, a defensive suggestion for an unreachable
wait<=0case, a minor doc-wording ambiguity) — none required changes.