From e48f3a924babf87dfc5035b289a1f1878bba6896 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arnaud=20He=CC=81ritier?= Date: Sat, 13 Jun 2026 23:51:08 +0200 Subject: [PATCH 1/3] feat(tui): show live status on transfer_task delegation card MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The transfer_task card hardcoded ✓, so an in-flight delegation looked finished while the sub-agent was still running. Drive the leading glyph from ToolStatus instead: animate the spinner while Running/Pending, show ✓ on completion and ✗ on error. Keep the icon a single fixed-width glyph (no elapsed-time suffix) so the wrapped task-text indent stays stable across statuses. Refs #3101 --- .../tool/transfertask/transfertask.go | 25 ++++- .../tool/transfertask/transfertask_test.go | 96 +++++++++++++++++++ 2 files changed, 118 insertions(+), 3 deletions(-) create mode 100644 pkg/tui/components/tool/transfertask/transfertask_test.go diff --git a/pkg/tui/components/tool/transfertask/transfertask.go b/pkg/tui/components/tool/transfertask/transfertask.go index 9f958de34..66d83d072 100644 --- a/pkg/tui/components/tool/transfertask/transfertask.go +++ b/pkg/tui/components/tool/transfertask/transfertask.go @@ -19,7 +19,7 @@ func New(msg *types.Message, sessionState service.SessionStateReader) layout.Mod return toolcommon.NewBase(msg, sessionState, render) } -func render(msg *types.Message, _ spinner.Spinner, _ service.SessionStateReader, width, _ int) string { +func render(msg *types.Message, s spinner.Spinner, _ service.SessionStateReader, width, _ int) string { var params transfertask.Args if err := json.Unmarshal([]byte(msg.ToolCall.Function.Arguments), ¶ms); err != nil { return "" @@ -29,8 +29,9 @@ func render(msg *types.Message, _ spinner.Spinner, _ service.SessionStateReader, " calls " + styles.AgentBadgeStyleFor(params.Agent).Render(params.Agent) - // Calculate the icon with its margin - icon := styles.ToolCompletedIcon.Render("✓") + // Status-aware icon: spinner while the delegation runs, ✓ on success, ✗ on error. + // Single glyph (no elapsed suffix) keeps the wrap math below stable. + icon := statusIcon(msg, s) iconWithSpace := icon + " " iconWidth := lipgloss.Width(iconWithSpace) @@ -57,3 +58,21 @@ func render(msg *types.Message, _ spinner.Spinner, _ service.SessionStateReader, return header + "\n\n" + taskContent.String() } + +// statusIcon picks the leading glyph for the delegation card from the tool +// status: an animated spinner while the sub-agent runs, ✓ on success, ✗ on +// error. The transfertask Base spinner is ModeSpinnerOnly, so s.View() is a +// single 1-cell glyph whose width matches ✓/✗, keeping the task-text wrap math +// stable (no elapsed-time suffix, unlike toolcommon.Icon). +func statusIcon(msg *types.Message, s spinner.Spinner) string { + switch msg.ToolStatus { + case types.ToolStatusRunning, types.ToolStatusPending, types.ToolStatusConfirmation: + return styles.NoStyle.MarginLeft(2).Render(s.View()) + case types.ToolStatusCompleted: + return styles.ToolCompletedIcon.Render("✓") + case types.ToolStatusError: + return styles.ToolErrorIcon.Render("✗") + default: // genuinely unknown/terminal states + return styles.ToolCompletedIcon.Render("✓") + } +} diff --git a/pkg/tui/components/tool/transfertask/transfertask_test.go b/pkg/tui/components/tool/transfertask/transfertask_test.go new file mode 100644 index 000000000..4a745b324 --- /dev/null +++ b/pkg/tui/components/tool/transfertask/transfertask_test.go @@ -0,0 +1,96 @@ +package transfertask + +import ( + "regexp" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/docker/docker-agent/pkg/tools" + "github.com/docker/docker-agent/pkg/tui/types" +) + +var ansiEscape = regexp.MustCompile("\x1b\\[[0-9;]*m") + +func stripANSI(s string) string { + return ansiEscape.ReplaceAllString(s, "") +} + +// spinnerGlyph is the first frame of the shared braille spinner. An in-flight +// delegation must show it instead of a success/error glyph. +const spinnerGlyph = "⠋" + +func transferMessage(status types.ToolStatus) *types.Message { + return &types.Message{ + Type: types.MessageTypeToolCall, + Sender: "root", + ToolStatus: status, + ToolCall: tools.ToolCall{ + Function: tools.FunctionCall{ + Arguments: `{"agent":"researcher","task":"Investigate the flaky test"}`, + }, + }, + } +} + +func renderCard(status types.ToolStatus) string { + view := New(transferMessage(status), nil) + view.SetSize(80, 1) + return stripANSI(view.View()) +} + +// TestTransferTaskCard_StatusIcon locks in the status-aware icon: a running or +// pending delegation animates the spinner (never showing a premature ✓), while +// completed/error terminal states show ✓/✗. The parent→child header and the +// task text must render in every state. +func TestTransferTaskCard_StatusIcon(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + status types.ToolStatus + want string // glyph that must be present + notWant []string // glyphs that must be absent + }{ + {"running animates spinner", types.ToolStatusRunning, spinnerGlyph, []string{"✓", "✗"}}, + {"pending animates spinner", types.ToolStatusPending, spinnerGlyph, []string{"✓", "✗"}}, + {"completed shows check", types.ToolStatusCompleted, "✓", []string{spinnerGlyph, "✗"}}, + {"error shows cross", types.ToolStatusError, "✗", []string{spinnerGlyph, "✓"}}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + out := renderCard(tc.status) + + assert.Regexp(t, `root\s+calls\s+researcher`, out, "header should name parent and child agents") + assert.Contains(t, out, "Investigate the flaky test", "task text should always render") + assert.Contains(t, out, tc.want, "expected status glyph missing") + for _, n := range tc.notWant { + assert.NotContains(t, out, n, "unexpected glyph %q present for %s", n, tc.name) + } + }) + } +} + +// TestTransferTaskCard_IconWidthIsStable guards the A.1 fixed-width contract: +// the icon stays a single glyph with no elapsed-time suffix, so the wrapped +// task text keeps the same indent across statuses. Swapping in an elapsed-time +// icon (e.g. toolcommon.Icon) would shift the running task column and regress. +func TestTransferTaskCard_IconWidthIsStable(t *testing.T) { + t.Parallel() + + taskColumn := func(status types.ToolStatus) int { + // Layout is "header\n\n task", so the task block is the 3rd part. + parts := strings.SplitN(renderCard(status), "\n", 3) + return strings.Index(parts[len(parts)-1], "Investigate") + } + + running := taskColumn(types.ToolStatusRunning) + assert.Positive(t, running, "task text should be indented past the icon") + assert.Equal(t, running, taskColumn(types.ToolStatusPending)) + assert.Equal(t, running, taskColumn(types.ToolStatusCompleted)) + assert.Equal(t, running, taskColumn(types.ToolStatusError)) +} From ba302f6620fa38547ab7c83a2dbd93b79eba95ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arnaud=20He=CC=81ritier?= Date: Sat, 13 Jun 2026 23:51:54 +0200 Subject: [PATCH 2/3] =?UTF-8?q?feat(tui):=20label=20the=20delegation=20spi?= =?UTF-8?q?nner=20with=20parent=20=E2=86=92=20child?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While the root agent waited on a sub-agent, the pending spinner showed an anonymous whimsical word with no hint of who was working. Track an agentStack in lockstep with streamDepth (pushed on StreamStarted, popped on StreamStopped, reset alongside streamDepth on cancel/submit) and, for delegated streams only (depth ≥ 2), render the spinner as an animated glyph plus "parent → child" in the child's accent color via a new types.SpinnerLabeled. The normal top-level turn keeps the playful spinner. messages.AddAssistantMessage now takes (sender, label) to carry this context through to the rendered spinner. Refs #3101 --- pkg/tui/components/message/message.go | 7 +++- pkg/tui/components/message/message_test.go | 31 +++++++++++++++++ pkg/tui/components/messages/leak_test.go | 2 +- pkg/tui/components/messages/messages.go | 10 ++++-- pkg/tui/page/chat/chat.go | 19 +++++++++-- pkg/tui/page/chat/runtime_events.go | 15 +++++++-- pkg/tui/page/chat/spinner_context_test.go | 39 ++++++++++++++++++++++ pkg/tui/types/types.go | 7 ++++ 8 files changed, 121 insertions(+), 9 deletions(-) create mode 100644 pkg/tui/page/chat/spinner_context_test.go diff --git a/pkg/tui/components/message/message.go b/pkg/tui/components/message/message.go index bd1f35237..1818f7952 100644 --- a/pkg/tui/components/message/message.go +++ b/pkg/tui/components/message/message.go @@ -254,7 +254,12 @@ func (mv *messageModel) render(width int) string { msg := mv.message switch msg.Type { case types.MessageTypeSpinner: - return mv.spinner.View() + if msg.Content == "" { + return mv.spinner.View() // top-level: keep the playful spinner + } + // Delegated stream: animated glyph + per-agent-colored "parent → child". + glyph := styles.SpinnerDotsAccentStyle.MarginLeft(2).Render(mv.spinner.RawFrame()) + return glyph + " " + styles.AgentAccentStyleFor(msg.Sender).Render(msg.Content) case types.MessageTypeUser: // Choose style based on selection state messageStyle := styles.UserMessageStyle diff --git a/pkg/tui/components/message/message_test.go b/pkg/tui/components/message/message_test.go index bcb955605..198a7002d 100644 --- a/pkg/tui/components/message/message_test.go +++ b/pkg/tui/components/message/message_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/docker/docker-agent/pkg/tui/components/spinner" "github.com/docker/docker-agent/pkg/tui/types" ) @@ -293,3 +294,33 @@ func TestUserMessageNotCollapsible(t *testing.T) { height := mv.Height(80) assert.False(t, mv.IsToggleLine(height-1)) } + +// TestLabeledSpinnerRendersDelegationContext covers the delegated-stream spinner: +// a MessageTypeSpinner carrying a label renders an animated glyph plus the +// "parent → child" text, and stays spinner-driven so it is never cached. +func TestLabeledSpinnerRendersDelegationContext(t *testing.T) { + t.Parallel() + + // Sender drives the accent color (child); Content holds the label. + msg := types.SpinnerLabeled("researcher", "root → researcher") + mv := New(msg, nil) + mv.SetSize(80, 0) + + assert.True(t, mv.isSpinnerDriven(), "labeled spinner must stay uncached/animated") + + out := stripANSI(mv.View()) + assert.Contains(t, out, "root → researcher", "label should read parent → child") + assert.Contains(t, out, spinner.Frame(0), "animated glyph should lead the label") +} + +// TestBareSpinnerKeepsPlayfulView ensures the normal top-level turn (empty +// label) is untouched: it still renders the playful spinner verbatim. +func TestBareSpinnerKeepsPlayfulView(t *testing.T) { + t.Parallel() + + mv := New(types.Spinner(), nil) + mv.SetSize(80, 0) + + assert.True(t, mv.isSpinnerDriven()) + assert.Equal(t, mv.spinner.View(), mv.View(), "empty label must keep the default spinner rendering") +} diff --git a/pkg/tui/components/messages/leak_test.go b/pkg/tui/components/messages/leak_test.go index a0a980979..b61d41135 100644 --- a/pkg/tui/components/messages/leak_test.go +++ b/pkg/tui/components/messages/leak_test.go @@ -75,7 +75,7 @@ func TestLongSessionDoesNotRetainPerMessageRenderState(t *testing.T) { streamMessage := func(i int) { m.AddUserMessage(fmt.Sprintf("user message %d", i)) - m.AddAssistantMessage() + m.AddAssistantMessage("", "") for off := 0; off < len(body); off += chunkSize { end := min(off+chunkSize, len(body)) m.AppendToLastMessage("root", body[off:end]) diff --git a/pkg/tui/components/messages/messages.go b/pkg/tui/components/messages/messages.go index 9655541ea..17faf6720 100644 --- a/pkg/tui/components/messages/messages.go +++ b/pkg/tui/components/messages/messages.go @@ -55,7 +55,7 @@ type Model interface { AddLoadingMessage(description string) tea.Cmd ReplaceLoadingWithUser(content string, sessionPos int) tea.Cmd AddErrorMessage(content string) tea.Cmd - AddAssistantMessage() tea.Cmd + AddAssistantMessage(sender, label string) tea.Cmd AddCancelledMessage() tea.Cmd AddWelcomeMessage(content string) tea.Cmd AddOrUpdateToolCall(agentName string, toolCall tools.ToolCall, toolDef tools.Tool, status types.ToolStatus) tea.Cmd @@ -1237,8 +1237,12 @@ func (m *model) AddShellOutputMessage(content string) tea.Cmd { return m.addMessage(types.ShellOutput(content)) } -func (m *model) AddAssistantMessage() tea.Cmd { - return m.addMessage(types.Spinner()) +func (m *model) AddAssistantMessage(sender, label string) tea.Cmd { + m.removeSpinner() + if label == "" { + return m.addMessage(types.Spinner()) + } + return m.addMessage(types.SpinnerLabeled(sender, label)) } func (m *model) AddCancelledMessage() tea.Cmd { diff --git a/pkg/tui/page/chat/chat.go b/pkg/tui/page/chat/chat.go index e48990ff8..e5df89dfb 100644 --- a/pkg/tui/page/chat/chat.go +++ b/pkg/tui/page/chat/chat.go @@ -142,7 +142,8 @@ type chatPage struct { msgCancel context.CancelFunc streamCancelled bool - streamDepth int // nesting depth of active streams (incremented on StreamStarted, decremented on StreamStopped) + streamDepth int // nesting depth of active streams (incremented on StreamStarted, decremented on StreamStopped) + agentStack []string // agent per active stream level; len(agentStack)==streamDepth streamStartTime time.Time // Track whether we've received content from an assistant response @@ -443,12 +444,24 @@ func (p *chatPage) setWorking(working bool) tea.Cmd { // the scrollable list; when stopping, it explicitly removes any lingering spinner. func (p *chatPage) setPendingResponse(pending bool) tea.Cmd { if pending { - return p.messages.AddAssistantMessage() + sender, label := p.pendingSpinnerContext() + return p.messages.AddAssistantMessage(sender, label) } p.messages.RemoveSpinner() return nil } +// pendingSpinnerContext labels the waiting spinner during delegation only. +// Depth < 2 → empty (default playful spinner); nested → child + "parent → child". +func (p *chatPage) pendingSpinnerContext() (sender, label string) { + n := len(p.agentStack) + if n < 2 { + return "", "" + } + child := p.agentStack[n-1] + return child, p.agentStack[n-2] + " → " + child +} + // renderCollapsedSidebar renders the sidebar in collapsed mode (at top of screen). func (p *chatPage) renderCollapsedSidebar(sl sidebarLayout) string { // Guard against unset/invalid layout (can happen before WindowSizeMsg is received). @@ -620,6 +633,7 @@ func (p *chatPage) cancelStream(showCancelMessage bool) tea.Cmd { p.msgCancel = nil p.streamCancelled = true p.streamDepth = 0 + p.agentStack = nil p.setPendingResponse(false) // Send StreamCancelledMsg to all components to handle cleanup return tea.Batch( @@ -890,6 +904,7 @@ func (p *chatPage) processMessage(msg msgtypes.SendMsg) tea.Cmd { } p.streamDepth = 0 + p.agentStack = nil p.sidebar.ResetStreamTracking() var ctx context.Context diff --git a/pkg/tui/page/chat/runtime_events.go b/pkg/tui/page/chat/runtime_events.go index 21d4f8d6b..c265b105e 100644 --- a/pkg/tui/page/chat/runtime_events.go +++ b/pkg/tui/page/chat/runtime_events.go @@ -202,6 +202,7 @@ func (p *chatPage) handleStreamStarted(msg *runtime.StreamStartedEvent) tea.Cmd slog.Debug("handleStreamStarted called", "agent", msg.AgentName, "session_id", msg.SessionID) p.streamCancelled = false p.streamDepth++ + p.agentStack = append(p.agentStack, msg.AgentName) p.streamStartTime = time.Now() spinnerCmd := p.setWorking(true) pendingCmd := p.setPendingResponse(true) @@ -233,12 +234,18 @@ func (p *chatPage) handleStreamStopped(msg *runtime.StreamStoppedEvent) tea.Cmd "agent", msg.AgentName, "session_id", msg.SessionID, "reason", msg.Reason, - "should_exit", p.app.ShouldExitAfterFirstResponse(), + "should_exit", p.app != nil && p.app.ShouldExitAfterFirstResponse(), "has_content", p.hasReceivedAssistantContent, "stream_depth", p.streamDepth) if p.streamDepth > 0 { p.streamDepth-- + // Keep agentStack in sync: only pop when there was a depth to decrement, + // so spurious/duplicate StreamStopped events at depth 0 cannot cause + // the two slices to diverge. + if n := len(p.agentStack); n > 0 { + p.agentStack = p.agentStack[:n-1] + } } sidebarCmd := p.forwardToSidebar(msg) @@ -247,8 +254,12 @@ func (p *chatPage) handleStreamStopped(msg *runtime.StreamStoppedEvent) tea.Cmd // forward to the sidebar and keep the working/cancel state intact. // Without this guard, pressing Esc after a sub-agent completes but // while the parent continues would have no effect. + // Also clear the now-stale "parent → child" labeled spinner and + // replace it with a plain parent spinner so the UI reflects the + // updated delegation state. if p.streamDepth > 0 { - return tea.Batch(p.messages.ScrollToBottom(), sidebarCmd) + p.setPendingResponse(false) + return tea.Batch(p.messages.ScrollToBottom(), sidebarCmd, p.setPendingResponse(true)) } // Outermost stream stopped — fully clean up. diff --git a/pkg/tui/page/chat/spinner_context_test.go b/pkg/tui/page/chat/spinner_context_test.go new file mode 100644 index 000000000..9d9a5c9b4 --- /dev/null +++ b/pkg/tui/page/chat/spinner_context_test.go @@ -0,0 +1,39 @@ +package chat + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// TestPendingSpinnerContext verifies the waiting-spinner label is scoped to +// delegated streams: depth < 2 keeps the default playful spinner (empty +// sender/label), while nested streams name the nearest parent → child pair and +// expose the child as the accent-color sender. +func TestPendingSpinnerContext(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + stack []string + wantSender string + wantLabel string + }{ + {"depth 0 - no stream", nil, "", ""}, + {"depth 1 - top-level turn", []string{"root"}, "", ""}, + {"depth 2 - single delegation", []string{"root", "researcher"}, "researcher", "root → researcher"}, + {"depth 3 - nested delegation", []string{"root", "researcher", "librarian"}, "librarian", "researcher → librarian"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + p := &chatPage{agentStack: tc.stack} + sender, label := p.pendingSpinnerContext() + + assert.Equal(t, tc.wantSender, sender, "sender (accent agent)") + assert.Equal(t, tc.wantLabel, label, "parent → child label") + }) + } +} diff --git a/pkg/tui/types/types.go b/pkg/tui/types/types.go index 61def0cb6..c37d1c3e6 100644 --- a/pkg/tui/types/types.go +++ b/pkg/tui/types/types.go @@ -83,6 +83,13 @@ func Spinner() *Message { } } +// SpinnerLabeled is a pending-response spinner that names the agent we're waiting +// on. Sender drives the accent color; Content holds the label (e.g. "root → x"). +// Empty Content renders the default spinner. +func SpinnerLabeled(sender, label string) *Message { + return &Message{Type: MessageTypeSpinner, Sender: sender, Content: label} +} + func Error(content string) *Message { return &Message{ Type: MessageTypeError, From 8ff2ff1d91845dc604778da29386f7c8eaa07291 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arnaud=20He=CC=81ritier?= Date: Mon, 15 Jun 2026 14:34:06 +0200 Subject: [PATCH 3/3] chore(lint): drop unused nolint:revive directive in elicitation_test.go