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/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)) +} 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,