diff --git a/pkg/tui/components/message/message.go b/pkg/tui/components/message/message.go index 1818f7952..c37147a79 100644 --- a/pkg/tui/components/message/message.go +++ b/pkg/tui/components/message/message.go @@ -375,6 +375,13 @@ func (mv *messageModel) render(width int) string { return msg.Content case types.MessageTypeCancelled: return styles.WarningStyle.Render("⚠ stream cancelled ⚠") + case types.MessageTypeDelegationReturn: + label := "↩ " + msg.Sender + " → " + msg.Content + styled := styles.AgentAccentStyleFor(msg.Sender).Render(label) + // Trailing faded rule out to width, mirroring the collapsed-sidebar divider. + ruleW := max(width-ansi.StringWidth(styled)-3, 0) + rule := styles.FadingStyle.Render(" " + strings.Repeat("─", ruleW)) + return styles.NoStyle.MarginLeft(2).Render(styled + rule) case types.MessageTypeWelcome: messageStyle := styles.WelcomeMessageStyle // Convert explicit newlines to markdown hard line breaks (two trailing spaces) diff --git a/pkg/tui/components/message/message_test.go b/pkg/tui/components/message/message_test.go index 198a7002d..4f5de68ae 100644 --- a/pkg/tui/components/message/message_test.go +++ b/pkg/tui/components/message/message_test.go @@ -324,3 +324,21 @@ func TestBareSpinnerKeepsPlayfulView(t *testing.T) { assert.True(t, mv.isSpinnerDriven()) assert.Equal(t, mv.spinner.View(), mv.View(), "empty label must keep the default spinner rendering") } + +// TestDelegationReturnMarkerRenders covers the transcript return marker inserted +// when a forwarded sub-agent (child) hands control back to its caller (parent): +// it renders "↩ child → parent" and, being static, is NOT spinner-driven so it +// caches normally. +func TestDelegationReturnMarkerRenders(t *testing.T) { + t.Parallel() + + // Sender holds the child (drives the accent color); Content holds the parent. + msg := types.DelegationReturn("librarian", "root") + mv := New(msg, nil) + mv.SetSize(80, 0) + + assert.False(t, mv.isSpinnerDriven(), "static return marker must be cacheable, not spinner-driven") + + out := stripANSI(mv.View()) + assert.Contains(t, out, "↩ librarian → root", "marker should read ↩ child → parent") +} diff --git a/pkg/tui/components/messages/messages.go b/pkg/tui/components/messages/messages.go index 17faf6720..8a13fb1e4 100644 --- a/pkg/tui/components/messages/messages.go +++ b/pkg/tui/components/messages/messages.go @@ -57,6 +57,7 @@ type Model interface { AddErrorMessage(content string) tea.Cmd AddAssistantMessage(sender, label string) tea.Cmd AddCancelledMessage() tea.Cmd + AddDelegationReturn(child, parent string) tea.Cmd AddWelcomeMessage(content string) tea.Cmd AddOrUpdateToolCall(agentName string, toolCall tools.ToolCall, toolDef tools.Tool, status types.ToolStatus) tea.Cmd AppendToolOutput(msg *runtime.ToolCallOutputEvent) tea.Cmd @@ -1025,6 +1026,9 @@ func (m *model) shouldCacheMessage(index int) bool { return false case types.MessageTypeUser: return true + case types.MessageTypeDelegationReturn: + // Static divider line; safe to cache like a user message. + return true default: return false } @@ -1255,6 +1259,10 @@ func (m *model) AddCancelledMessage() tea.Cmd { return view.Init() } +func (m *model) AddDelegationReturn(child, parent string) tea.Cmd { + return m.addMessage(types.DelegationReturn(child, parent)) +} + func (m *model) AddWelcomeMessage(content string) tea.Cmd { if content == "" || len(m.views) > 0 { return nil diff --git a/pkg/tui/components/messages/messages_test.go b/pkg/tui/components/messages/messages_test.go index 0af86fc6b..c5840ffe3 100644 --- a/pkg/tui/components/messages/messages_test.go +++ b/pkg/tui/components/messages/messages_test.go @@ -39,6 +39,27 @@ func TestViewDoesNotWrapWideLines(t *testing.T) { } } +// TestAddDelegationReturnAppendsMarker verifies AddDelegationReturn appends a +// single MessageTypeDelegationReturn item (Sender=child, Content=parent) and +// that it renders the "↩ child → parent" divider. +func TestAddDelegationReturnAppendsMarker(t *testing.T) { + t.Parallel() + + sessionState := &service.SessionState{} + m := NewScrollableView(80, 24, sessionState).(*model) + m.SetSize(80, 24) + + cmd := m.AddDelegationReturn("librarian", "root") + require.NotNil(t, cmd, "appending the marker auto-scrolls a fresh transcript, yielding a cmd") + + require.Len(t, m.messages, 1) + assert.Equal(t, types.MessageTypeDelegationReturn, m.messages[0].Type) + assert.Equal(t, "librarian", m.messages[0].Sender) + assert.Equal(t, "root", m.messages[0].Content) + + assert.Contains(t, ansi.Strip(m.View()), "↩ librarian → root") +} + func TestMouseClickOnURLOpensURL(t *testing.T) { t.Parallel() diff --git a/pkg/tui/components/sidebar/delegation_breadcrumb_test.go b/pkg/tui/components/sidebar/delegation_breadcrumb_test.go new file mode 100644 index 000000000..ed0a058c8 --- /dev/null +++ b/pkg/tui/components/sidebar/delegation_breadcrumb_test.go @@ -0,0 +1,153 @@ +package sidebar + +import ( + "strings" + "testing" + + "github.com/charmbracelet/x/ansi" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/runtime" + "github.com/docker/docker-agent/pkg/session" + "github.com/docker/docker-agent/pkg/tui/messages" + "github.com/docker/docker-agent/pkg/tui/service" +) + +func newBreadcrumbSidebar(t *testing.T) *model { + t.Helper() + sess := session.New() + sessionState := service.NewSessionState(sess) + sessionState.SetCurrentAgentName("root") + + m := New(sessionState).(*model) + // Driving the real Update handlers starts the model's spinner, which registers + // with the process-global animation coordinator. Release it on cleanup so a + // leaked registration can't make HasActive() true for other sidebar tests + // (e.g. TestSidebar_TitleRegenerating asserts the first animation is active). + t.Cleanup(func() { m.spinner.Stop() }) + m.sessionHasContent = true + m.titleGenerated = true + m.sessionTitle = "Test" + m.currentAgent = "root" + m.availableAgents = []runtime.AgentDetails{ + {Name: "root", Provider: "openai", Model: "gpt-4o", Description: "Orchestrator"}, + {Name: "librarian", Provider: "openai", Model: "gpt-4o", Description: "Finds documents"}, + } + m.width = 60 + m.height = 60 + return m +} + +func streamStarted(agent, sessionID string) *runtime.StreamStartedEvent { + return &runtime.StreamStartedEvent{ + AgentContext: runtime.AgentContext{AgentName: agent}, + SessionID: sessionID, + } +} + +// TestAgentChainTracksSessionStack enforces the invariant +// len(agentChain) == len(sessionStack): the chain is pushed on StreamStarted and +// popped on StreamStopped, in lockstep with the session stack. +// +// Not parallel: it drives the real Update handlers, which start the spinner and +// touch the process-global animation coordinator shared across tests. +func TestAgentChainTracksSessionStack(t *testing.T) { + m := newBreadcrumbSidebar(t) + require.Len(t, m.agentChain, len(m.sessionStack)) + + m.Update(streamStarted("root", "s-root")) + assert.Equal(t, []string{"root"}, m.agentChain) + assert.Len(t, m.agentChain, len(m.sessionStack)) + + m.Update(streamStarted("librarian", "s-lib")) + assert.Equal(t, []string{"root", "librarian"}, m.agentChain) + assert.Len(t, m.agentChain, len(m.sessionStack)) + + m.Update(&runtime.StreamStoppedEvent{SessionID: "s-lib"}) + assert.Equal(t, []string{"root"}, m.agentChain) + assert.Len(t, m.agentChain, len(m.sessionStack)) + + m.Update(&runtime.StreamStoppedEvent{SessionID: "s-root"}) + assert.Empty(t, m.agentChain) + assert.Len(t, m.agentChain, len(m.sessionStack)) +} + +// Not parallel: drives the real Update handlers, which touch the process-global +// animation coordinator shared across tests. +func TestAgentChainResets(t *testing.T) { + t.Run("StreamCancelledMsg clears the chain", func(t *testing.T) { + m := newBreadcrumbSidebar(t) + m.Update(streamStarted("root", "s-root")) + m.Update(streamStarted("librarian", "s-lib")) + + m.Update(messages.StreamCancelledMsg{}) + + assert.Empty(t, m.agentChain) + assert.Empty(t, m.sessionStack) + }) + + t.Run("ResetStreamTracking clears the chain", func(t *testing.T) { + m := newBreadcrumbSidebar(t) + m.Update(streamStarted("root", "s-root")) + m.Update(streamStarted("librarian", "s-lib")) + + m.ResetStreamTracking() + + assert.Empty(t, m.agentChain) + assert.Empty(t, m.sessionStack) + }) +} + +// TestDelegationBreadcrumbRendersOnlyWhenNested verifies the breadcrumb shows the +// active chain (e.g. "root ⏵ librarian") only once delegation depth exceeds 1. +func TestDelegationBreadcrumbRendersOnlyWhenNested(t *testing.T) { + t.Parallel() + + t.Run("hidden at depth <= 1", func(t *testing.T) { + t.Parallel() + m := newBreadcrumbSidebar(t) + m.agentChain = []string{"root"} + assert.NotContains(t, ansi.Strip(m.View()), "⏵") + }) + + t.Run("shown at depth > 1", func(t *testing.T) { + t.Parallel() + m := newBreadcrumbSidebar(t) + m.agentChain = []string{"root", "librarian"} + assert.Contains(t, ansi.Strip(m.View()), "root ⏵ librarian") + }) +} + +// TestDelegationBreadcrumbPreservesAgentClickZones guards the buildAgentClickZones +// skip logic: the breadcrumb block agentInfo prepends must not become a click +// zone or shift the per-agent rows. Without the skip, the breadcrumb would claim +// the first agent slot and the current-agent row would mis-map. +func TestDelegationBreadcrumbPreservesAgentClickZones(t *testing.T) { + t.Parallel() + + m := newBreadcrumbSidebar(t) + m.agentChain = []string{"root", "librarian"} + + _ = m.View() // populate agentClickZones + cachedLines + + for i, line := range m.cachedLines { + stripped := ansi.Strip(line) + // The breadcrumb (joined by ⏵) must never be an agent click target. + if strings.Contains(stripped, "⏵") { + _, isZone := m.agentClickZones[i] + assert.False(t, isZone, "breadcrumb line %d must not be a click zone", i) + } + // The current-agent roster row (prefixed with ▶) must map to root. + if strings.Contains(stripped, "▶") { + assert.Equal(t, "root", m.agentClickZones[i], "current-agent row %d should map to root", i) + } + } + + var clickable []string + for _, name := range m.agentClickZones { + clickable = append(clickable, name) + } + assert.Contains(t, clickable, "root") + assert.Contains(t, clickable, "librarian") +} diff --git a/pkg/tui/components/sidebar/sidebar.go b/pkg/tui/components/sidebar/sidebar.go index 6acebcf3e..c281df11d 100644 --- a/pkg/tui/components/sidebar/sidebar.go +++ b/pkg/tui/components/sidebar/sidebar.go @@ -137,6 +137,7 @@ type model struct { sessionState *service.SessionState workingAgent string // Name of the agent currently working (empty if none) sessionStack []string // Active stream session IDs; the top is the active (deepest) session + agentChain []string // Agent name per active stream level; invariant: len == len(sessionStack) rootSessionID string // Main (top-level) session, shown when no stream is active scrollview *scrollview.Model workingDirectory string @@ -477,6 +478,7 @@ func (m *model) LoadFromSession(sess *session.Session) { // loaded session has no in-flight streams, so clear any stale stack entries. m.rootSessionID = sess.ID m.sessionStack = nil + m.agentChain = nil // Load session title if sess.Title != "" { @@ -509,6 +511,7 @@ func (m *model) ResetStreamTracking() { return } m.sessionStack = nil + m.agentChain = nil m.invalidateCache() } @@ -736,6 +739,7 @@ func (m *model) Update(msg tea.Msg) (layout.Model, tea.Cmd) { m.rootSessionID = msg.SessionID } m.sessionStack = append(m.sessionStack, msg.SessionID) + m.agentChain = append(m.agentChain, msg.AgentName) // If title hasn't been generated yet, show the title generation spinner if !m.titleGenerated { m.titleRegenerating = true @@ -748,6 +752,9 @@ func (m *model) Update(msg tea.Msg) (layout.Model, tea.Cmd) { if n := len(m.sessionStack); n > 0 { m.sessionStack = m.sessionStack[:n-1] } + if n := len(m.agentChain); n > 0 { + m.agentChain = m.agentChain[:n-1] + } m.invalidateCache() m.stopSpinner() // Will only stop if no other state needs it return m, nil @@ -777,6 +784,7 @@ func (m *model) Update(msg tea.Msg) (layout.Model, tea.Cmd) { m.streamCancelled = true m.workingAgent = "" m.sessionStack = nil + m.agentChain = nil m.toolsLoading = false m.mcpInit = false m.titleRegenerating = false @@ -1250,6 +1258,9 @@ func (m *model) agentInfo(contentWidth int) string { } var content strings.Builder + if breadcrumb := m.delegationBreadcrumb(contentWidth); breadcrumb != "" { + content.WriteString(breadcrumb) + } for i, agent := range m.availableAgents { if content.Len() > 0 { content.WriteString("\n\n") @@ -1261,6 +1272,47 @@ func (m *model) agentInfo(contentWidth int) string { return m.renderTab(agentTitle, content.String(), contentWidth) } +// delegationBreadcrumb renders the active delegation chain (e.g. +// "root ⏵ librarian") shown under the Agents title while a sub-agent runs. It +// returns "" unless the chain is deeper than the root (len > 1). When the full +// chain would exceed contentWidth the middle is elided as "root ⏵ … ⏵ leaf". +// +// TODO(#3103, Appendix C.2): append a muted "+N background" count here once a +// background-task snapshot is available. +func (m *model) delegationBreadcrumb(contentWidth int) string { + chain := m.agentChain + if len(chain) <= 1 { + return "" + } + + sep := styles.MutedStyle.Render(" ⏵ ") + colored := func(name string) string { return styles.AgentAccentStyleFor(name).Render(name) } + + var b strings.Builder + b.WriteString(colored(chain[0])) + for _, name := range chain[1:] { + b.WriteString(sep) + b.WriteString(colored(name)) + } + full := b.String() + if contentWidth <= 0 || ansi.StringWidth(full) <= contentWidth { + return full + } + + // Too wide: keep the root and the deepest agent, elide the middle. + elided := colored(chain[0]) + sep + styles.MutedStyle.Render("…") + sep + colored(chain[len(chain)-1]) + if ansi.StringWidth(elided) <= contentWidth { + return elided + } + return ansi.Truncate(full, contentWidth, "…") +} + +// hasDelegationBreadcrumb reports whether agentInfo prepends a delegation +// breadcrumb block; buildAgentClickZones uses it to keep click rows aligned. +func (m *model) hasDelegationBreadcrumb() bool { + return len(m.agentChain) > 1 +} + func (m *model) renderAgentEntry(content *strings.Builder, agent runtime.AgentDetails, isCurrent bool, index, contentWidth int) { agentStyle := styles.AgentAccentStyleFor(agent.Name) var prefix string @@ -1317,6 +1369,20 @@ func isVisuallyBlank(line string) bool { return strings.TrimSpace(ansi.Strip(line)) == "" } +// skipLeadingBlock returns the index just past the first run of non-blank lines +// and the blank separator that follows it. agentInfo emits the delegation +// breadcrumb as one such block above the roster; buildAgentClickZones skips it. +func skipLeadingBlock(lines []string, start int) int { + i := start + for i < len(lines) && !isVisuallyBlank(lines[i]) { + i++ + } + for i < len(lines) && isVisuallyBlank(lines[i]) { + i++ + } + return i +} + // buildAgentClickZones populates agentClickZones by scanning the rendered lines // to find which lines belong to which agent. It relies on the structure produced // by renderTab + agentInfo: a 2-line tab header, then agent blocks separated by @@ -1329,10 +1395,16 @@ func (m *model) buildAgentClickZones(agentSectionStart int, lines []string) { } const tabHeaderLines = 2 // tab title + TabStyle top padding + start := agentSectionStart + tabHeaderLines + // agentInfo may prepend a single-line delegation breadcrumb block above the + // roster; skip it (and its trailing blank) so click rows map to agents. + if m.hasDelegationBreadcrumb() { + start = skipLeadingBlock(lines, start) + } agentIdx := 0 inBlock := false - for i := agentSectionStart + tabHeaderLines; i < len(lines) && agentIdx < len(m.availableAgents); i++ { + for i := start; i < len(lines) && agentIdx < len(m.availableAgents); i++ { if isVisuallyBlank(lines[i]) { // Blank line: if we were inside a block, advance to the next agent if inBlock { diff --git a/pkg/tui/page/chat/runtime_events.go b/pkg/tui/page/chat/runtime_events.go index c265b105e..1e389a2c5 100644 --- a/pkg/tui/page/chat/runtime_events.go +++ b/pkg/tui/page/chat/runtime_events.go @@ -122,6 +122,11 @@ func (p *chatPage) handleRuntimeEvent(msg tea.Msg) (bool, tea.Cmd) { case *runtime.AgentSwitchingEvent: p.sidebar.SetAgentSwitching(msg.Switching) + if !msg.Switching && msg.FromAgent != "" && msg.ToAgent != "" { + // A forwarded sub-agent (FromAgent) just returned control to its + // caller (ToAgent); bracket the exit with a transcript marker. + return true, p.messages.AddDelegationReturn(msg.FromAgent, msg.ToAgent) + } return true, nil case *runtime.ToolsetInfoEvent: diff --git a/pkg/tui/page/chat/runtime_events_test.go b/pkg/tui/page/chat/runtime_events_test.go new file mode 100644 index 000000000..9af1bfa1d --- /dev/null +++ b/pkg/tui/page/chat/runtime_events_test.go @@ -0,0 +1,79 @@ +package chat + +import ( + "testing" + + "github.com/charmbracelet/x/ansi" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/runtime" + "github.com/docker/docker-agent/pkg/tui/components/messages" + "github.com/docker/docker-agent/pkg/tui/components/sidebar" + "github.com/docker/docker-agent/pkg/tui/service" +) + +func newDelegationMarkerTestPage() *chatPage { + sessionState := &service.SessionState{} + msgs := messages.NewScrollableView(80, 24, sessionState) + msgs.SetSize(80, 24) + return &chatPage{ + sidebar: sidebar.New(sessionState), + messages: msgs, + sessionState: sessionState, + } +} + +// TestAgentSwitchingInsertsReturnMarker covers the Phase 2 design decision to +// bracket only the EXIT of a forwarded sub-agent. AgentSwitching(false, child, +// parent) — emitted when a transfer_task/run_skill sub-session returns — inserts +// a "↩ child → parent" marker, while the entry event AgentSwitching(true, ...) +// inserts nothing (the transfer_task card is the visible entry). +func TestAgentSwitchingInsertsReturnMarker(t *testing.T) { + t.Parallel() + + t.Run("return (Switching=false) inserts a marker", func(t *testing.T) { + t.Parallel() + p := newDelegationMarkerTestPage() + + handled, cmd := p.handleRuntimeEvent(&runtime.AgentSwitchingEvent{ + Switching: false, + FromAgent: "librarian", + ToAgent: "root", + }) + + require.True(t, handled) + require.NotNil(t, cmd, "a returning sub-agent should insert a marker") + assert.Contains(t, ansi.Strip(p.messages.View()), "↩ librarian → root") + }) + + t.Run("entry (Switching=true) inserts nothing", func(t *testing.T) { + t.Parallel() + p := newDelegationMarkerTestPage() + + handled, cmd := p.handleRuntimeEvent(&runtime.AgentSwitchingEvent{ + Switching: true, + FromAgent: "root", + ToAgent: "librarian", + }) + + require.True(t, handled) + assert.Nil(t, cmd, "entering a delegation must not insert a return marker") + assert.NotContains(t, ansi.Strip(p.messages.View()), "↩") + }) + + t.Run("missing agent names insert nothing", func(t *testing.T) { + t.Parallel() + p := newDelegationMarkerTestPage() + + handled, cmd := p.handleRuntimeEvent(&runtime.AgentSwitchingEvent{ + Switching: false, + FromAgent: "", + ToAgent: "root", + }) + + require.True(t, handled) + assert.Nil(t, cmd) + assert.NotContains(t, ansi.Strip(p.messages.View()), "↩") + }) +} diff --git a/pkg/tui/types/types.go b/pkg/tui/types/types.go index c37d1c3e6..98273af8b 100644 --- a/pkg/tui/types/types.go +++ b/pkg/tui/types/types.go @@ -22,6 +22,7 @@ const ( MessageTypeToolResult MessageTypeWelcome MessageTypeLoading + MessageTypeDelegationReturn // "↩ child → parent" divider when a sub-agent returns ) const ( @@ -110,6 +111,13 @@ func Cancelled() *Message { } } +// DelegationReturn marks control returning from a sub-agent (child) to its +// caller (parent). Sender holds the child (drives the accent color); Content +// holds the parent name. +func DelegationReturn(child, parent string) *Message { + return &Message{Type: MessageTypeDelegationReturn, Sender: child, Content: parent} +} + func Welcome(content string) *Message { return &Message{ Type: MessageTypeWelcome,