fix(app): synthesize StreamStopped when the runtime stream closes without one

[aheritier]

🤖 Automated implementer agent — this comment was posted by the implementer bot from Docker Agentic Platform, not by a human developer

Fixes #4136 (TUI stuck on "Working…" after the stream ends).

Root cause (App-layer half)

The runtime documents the events channel close, not receipt of StreamStoppedEvent, as the terminal signal — the event is emitted non-blockingly at teardown (LocalRuntime.finalizeEventChannel) and is dropped under back-pressure (e.g. a fast local model streaming faster than the synchronous per-delta SQLite writes can keep up). The chat page, supervisor, and leantui only clear their "Working…" state on receipt of that event, so the TUI is stuck until Esc.

This traces back to the best-effort emit introduced by #3074/#3275. A separate, independent PR (Task A) fixes the runtime-side delivery guarantee in pkg/runtime; this PR is the App-layer half — honoring the runtime's own documented "channel close is terminal" contract.

Change

• forwardRunStreamEvents (used by Run/Retry/RunWithMessage) and RunSkillFork's drain loop now track whether a StreamStoppedEvent for the relevant session was actually observed during the drain, and synthesize+forward one when the channel closes without it.
• Reason is canceled if ctx.Err() != nil, else error if a root ErrorEvent was seen, else normal; agent name falls back to the last observed root-session agent name, then to Runtime.CurrentAgentName.
• Exports runtime.TurnEndReasonNormal/Error/Canceled (aliased internally) so app/TUI stop duplicating those string literals.

Testing

• TestForwardRunStreamEvents_SynthesizesRootStreamStopped: 5 scenarios (no stop, real stop, sub-session-only stop, ctx cancelled, root error) × all 3 entry points (Run/Retry/RunWithMessage).
• Two new RunSkillFork tests (synthesize-when-missing, no-duplicate-when-present).
• go build ./..., go test ./..., go test -race ./pkg/app/..., golangci-lint run all pass.

[rumpl]

The runtime documents the events channel close, not receipt of StreamStoppedEvent, as the terminal signal — the event is emitted non-blockingly at teardown (LocalRuntime.finalizeEventChannel) and is dropped under back-pressure (e.g. a fast local model streaming faster than the synchronous per-delta SQLite writes can keep up).

prove it

[aheritier]

🤖 Automated implementer agent — this comment was posted by the implementer bot from Docker Agentic Platform, not by a human developer

Three pieces of evidence:

1. The bug report's own logs (#4136 itself). The reporter's debug log shows the sequence: stopped=true → Conversation stopped → then nothing until they pressed Esc, at which point session_end hooks and on-user-input hooks fire (both of which finalizeEventChannel runs after attempting the StreamStopped emit). That gap between "conversation stopped" and "Esc pressed" is the TUI sitting on a channel read that never got the event.

2. The code's own documented contract, pkg/runtime/loop.go lines 177-207 (finalizeEventChannel):

Delivery: StreamStopped is best-effort. It is emitted non-blockingly and is dropped when the buffer is full and the consumer has gone away, rather than blocking teardown (a blocking send here is the deadlock #3070 fixed). Consumers must rely on the channel close, not on receiving StreamStopped, as the guaranteed terminal signal.

The actual emit: nonBlocking(&channelSink{ch: events}).Emit(StreamStopped(...)), where nonBlockingChannelSink.Emit is a literal select { case s.ch <- e: default: } — a full buffer means the default branch silently drops the event. The channel buffer is 128 (defaultEventChannelCapacity), so it fills whenever the consumer's drain loop falls behind the producer for even a short burst.

3. A deterministic repro I ran locally against this exact code path (not committed — a throwaway _test.go in pkg/runtime, deleted after running):

ch := make(chan Event, 2) // small buffer to force overflow
sink := nonBlocking(NewChannelSink(ch))
sink.Emit(StreamStarted("s", "a"))
sink.Emit(StreamStarted("s", "a")) // buffer now full
sink.Emit(StreamStopped("s", "a", "normal")) // this call: proof time
close(ch)
// draining ch: 2 StreamStartedEvents, zero StreamStoppedEvent

Output:

=== RUN   TestZZZProofNonBlockingSinkDropsOnFullBuffer
    zzz_proof_test.go:40: confirmed: StreamStopped was silently dropped when the channel buffer (cap=2) was full - channel closed with 2 events, none of them StreamStopped
--- PASS: TestZZZProofNonBlockingSinkDropsOnFullBuffer (0.00s)
PASS

A fast local model (vLLM, no network round-trip) streaming deltas faster than the consumer drains them (each delta triggers a synchronous SQLite write per the reporter's log timestamps) is exactly the burst that fills the 128-slot buffer and drops the StreamStopped emit — reproducing #4136.

[rumpl]

There is no way to make it not be best effort?

[aheritier]

🤖 Automated implementer agent — this comment was posted by the implementer bot from Docker Agentic Platform, not by a human developer

Yes — that's exactly what #4153 (companion PR, same effort) does: it changes finalizeEventChannel's StreamStopped emit from non-blocking/best-effort to a bounded blocking send (~5s deadline) in pkg/runtime/loop.go, so a consumer that's still draining the channel reliably receives it even under back-pressure. It's deliberately still bounded rather than unbounded, because an audit there found call sites that legitimately stop reading before close (a2a, acp, cli runner, LocalRuntime.Run, embeddedchat, server RunSession) — an unbounded send would reintroduce the #3070 teardown deadlock that made this emit non-blocking in the first place.

This PR is deliberately independent of that fix, as defense-in-depth: even with #4153's bounded send, an emit can still legitimately be dropped (consumer genuinely gone, deadline exceeded) or a future regression could reintroduce a non-blocking path — this PR makes the app layer honor the runtime's own documented "channel close is the guaranteed terminal signal" contract directly, so the TUI/supervisor can't get stuck regardless of whether the runtime's emit succeeds. The two together: #4153 makes delivery much more reliable, this PR makes correctness not depend on delivery at all.