fix(tui): keep pending dialog when a nested sub-agent stream starts#3221
Open
aheritier wants to merge 1 commit into
Open
fix(tui): keep pending dialog when a nested sub-agent stream starts#3221aheritier wants to merge 1 commit into
aheritier wants to merge 1 commit into
Conversation
docker-agent
left a comment
Contributor
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The fix is correct and well-targeted. isTopLevelStream cleanly guards both StreamStartedEvent and StreamStoppedEvent handlers so that nested sub-session stream lifecycle events (carrying a child SessionID) no longer wipe the parent runner's PendingEvent or toggle IsRunning. The empty-SessionID backward-compatibility path matches the existing convention in handleTokenUsage and is clearly documented. The six new tests cover the key cases — sub-session no-ops, top-level superseding stale events, and all three attention event types on StreamStopped.
No bugs were found in the introduced changes.
dgageot
previously approved these changes
Jun 24, 2026
dgageot
reviewed
Jun 24, 2026
dgageot
approved these changes
Jun 24, 2026
A user_prompt/elicitation (or tool-confirmation/max-iterations) dialog was silently dropped when a sub-agent or fork-skill stream started in the same session. The supervisor cleared the runner's PendingEvent on any StreamStarted/Stopped event regardless of session, so a forwarded nested sub-session stream wiped the parent's pending attention event, leaving the run blocked on user input with no visible dialog. Gate the StreamStarted/Stopped state mutations on a new isTopLevelStream helper so nested sub-session streams no longer toggle IsRunning or clear the parent runner's pending event, while top-level turns still supersede stale pending events. Fixes #3217
ec94df5 to
c29bc00
Compare
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 #3217.
A
user_prompt/elicitation (or tool-confirmation / max-iterations) dialog could be silently dropped when a sub-agent or fork-skill stream started in the same session, leaving the run blocked on user input with no visible dialog.Root cause
pkg/tui/service/supervisor/supervisor.go→handleRuntimeEventclearedrunner.PendingEvent(and toggledIsRunning/NeedsAttn) on anyStreamStarted/StreamStoppedevent, regardless of which session it belonged to.transfer_taskand fork-moderun_skillforward the child session's stream lifecycle events up through the parent's event channel; those nested events carry a differentSessionIDthan the runner key, so they wiped the parent runner's pending attention event.runner.PendingEventis the only thing that keeps a backgrounded attention dialog alive across a tab switch/re-render, so once cleared,ConsumePendingEventreturned nil, the dialog was never re-displayed, and the runtime stayed blocked inelicitationHandler.Fix
Add an
isTopLevelStream(runnerID, evSessionID string) boolhelper (evSessionID == "" || evSessionID == runnerID) and gate both theStreamStartedEventandStreamStoppedEventstate mutations on it. Nested sub-session streams no longer toggleIsRunningor clear the parent runner's pending event; top-level turns still supersede stale pending events exactly as before. An emptySessionIDis treated as top-level for backward compatibility (mirrors the existinghandleTokenUsageguard).This also corrects a latent tab-indicator bug: a nested
StreamStoppedno longer flips the parent tab'sIsRunningto false mid-turn.Tests
Added to
pkg/tui/service/supervisor/supervisor_test.go:TestIsTopLevelStream— table: matching / empty / child / sibling session IDsTestStreamStarted_SubSessionDoesNotDropPendingEventTestStreamStopped_SubSessionDoesNotDropPendingEventTestStreamStarted_TopLevelSupersedesStalePendingTestStreamStopped_TopLevelClearsPendingAndNeedsAttn— covers all three attention event types (elicitation, tool confirmation, max-iterations)TestStreamStarted_EmptySessionID_TreatedAsTopLevelValidation
task build✅task test✅ (only pre-existing, unrelated baseline failures)go test -race ./pkg/tui/service/supervisor/...✅task lint✅ (only pre-existingSlogContextualwarnings)