diff --git a/internal/app/app.go b/internal/app/app.go index c851a882..b611cd3d 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -24,7 +24,7 @@ import ( // // App satisfies the ui.AppController interface defined in internal/ui/model.go: // -// Run(prompt string) +// Run(prompt string) int // CancelCurrentStep() // QueueLength() int // ClearQueue() @@ -88,30 +88,36 @@ func (a *App) SetProgram(p *tea.Program) { // Run queues a prompt for execution. If the app is idle the prompt is // executed immediately in a background goroutine; otherwise it is appended -// to the queue and a QueueUpdatedEvent is sent to the program. +// to the queue. +// +// Returns the current queue depth after the operation: 0 means the prompt +// was started immediately (or the app is closed), >0 means it was queued. +// The caller is responsible for updating any UI state (e.g. queue badge) +// based on the returned value — Run does NOT send events to the program, +// because it may be called synchronously from within Bubble Tea's Update +// loop where prog.Send would deadlock. // // Satisfies ui.AppController. -func (a *App) Run(prompt string) { +func (a *App) Run(prompt string) int { a.mu.Lock() if a.closed { a.mu.Unlock() - return + return 0 } if a.busy { a.queue = append(a.queue, prompt) qLen := len(a.queue) a.mu.Unlock() - // sendEvent must be called without a.mu held (see sendEvent comment). - a.sendEvent(QueueUpdatedEvent{Length: qLen}) - return + return qLen } a.busy = true a.wg.Add(1) a.mu.Unlock() go a.drainQueue(prompt) + return 0 } // CancelCurrentStep cancels the currently executing agent step. Safe to call @@ -134,15 +140,16 @@ func (a *App) QueueLength() int { return len(a.queue) } -// ClearQueue discards all queued prompts and sends a QueueUpdatedEvent. +// ClearQueue discards all queued prompts. The caller is responsible for +// updating any UI state (e.g. queue badge) — ClearQueue does NOT send +// events to the program, because it may be called synchronously from +// within Bubble Tea's Update loop where prog.Send would deadlock. // // Satisfies ui.AppController. func (a *App) ClearQueue() { a.mu.Lock() a.queue = a.queue[:0] a.mu.Unlock() - // sendEvent must be called without a.mu held (see sendEvent comment). - a.sendEvent(QueueUpdatedEvent{Length: 0}) } // ClearMessages empties the conversation history. diff --git a/internal/ui/children_test.go b/internal/ui/children_test.go index d774598e..00481923 100644 --- a/internal/ui/children_test.go +++ b/internal/ui/children_test.go @@ -209,11 +209,13 @@ func TestInputComponent_ClearNilCtrl_NoPanic(t *testing.T) { } // -------------------------------------------------------------------------- -// TestInputComponent_ClearQueueCallsClearQueue verifies that /clear-queue (and -// its alias /cq) calls appCtrl.ClearQueue() and returns no submitMsg. +// TestInputComponent_ClearQueue_ForwardsAsSubmitMsg verifies that /clear-queue +// (and its alias /cq) are forwarded as submitMsg to the parent model (so the +// parent can call ClearQueue and update queueCount directly, avoiding a +// deadlock from calling prog.Send within Update). // -------------------------------------------------------------------------- -func TestInputComponent_ClearQueueCallsClearQueue(t *testing.T) { +func TestInputComponent_ClearQueue_ForwardsAsSubmitMsg(t *testing.T) { aliases := []string{"/clear-queue", "/cq"} for _, alias := range aliases { t.Run(alias, func(t *testing.T) { @@ -224,14 +226,21 @@ func TestInputComponent_ClearQueueCallsClearQueue(t *testing.T) { _, cmd := sendInputMsg(c, tea.KeyPressMsg{Code: tea.KeyEnter}) - if ctrl.clearQueueCalled != 1 { - t.Fatalf("%s: expected ClearQueue() called once, got %d", alias, ctrl.clearQueueCalled) + // ClearQueue should NOT be called directly by InputComponent. + if ctrl.clearQueueCalled != 0 { + t.Fatalf("%s: expected ClearQueue() not called, got %d", alias, ctrl.clearQueueCalled) } - if cmd != nil { - msg := runCmd(cmd) - if _, ok := msg.(submitMsg); ok { - t.Fatalf("%s: /clear-queue should not emit submitMsg, got submitMsg", alias) - } + // Instead, a submitMsg should be emitted so the parent handles it. + if cmd == nil { + t.Fatalf("%s: expected submitMsg cmd, got nil", alias) + } + msg := runCmd(cmd) + sm, ok := msg.(submitMsg) + if !ok { + t.Fatalf("%s: expected submitMsg, got %T", alias, msg) + } + if sm.Text != alias { + t.Fatalf("%s: expected submitMsg text %q, got %q", alias, alias, sm.Text) } }) } diff --git a/internal/ui/input.go b/internal/ui/input.go index 2d05271b..ee3826ab 100644 --- a/internal/ui/input.go +++ b/internal/ui/input.go @@ -17,7 +17,10 @@ import ( // Slash commands handled locally (not forwarded to app layer): // - /quit, /q, /exit → tea.Quit // - /clear, /cls, /c → appCtrl.ClearMessages() then clear the textarea -// - /clear-queue → appCtrl.ClearQueue() then clear the textarea +// +// /clear-queue is forwarded to the parent via submitMsg so the parent can +// update queueCount directly (calling ClearQueue from within Update would +// require prog.Send which deadlocks). // // All other input is returned via submitMsg for the parent to forward to // app.Run(). @@ -196,6 +199,10 @@ func (s *InputComponent) handleSubmit(value string) tea.Cmd { } // Resolve via canonical command lookup so aliases are handled uniformly. + // Only /quit and /clear are handled locally — /clear-queue must go + // through the parent model so it can update queueCount directly + // (calling ClearQueue here would skip the UI state update since we + // can't send events from within Update without deadlocking). if sc := GetCommandByName(trimmed); sc != nil { switch sc.Name { case "/quit": @@ -207,12 +214,6 @@ func (s *InputComponent) handleSubmit(value string) tea.Cmd { } // Don't forward to app.Run(); just clear silently. return nil - - case "/clear-queue": - if s.appCtrl != nil { - s.appCtrl.ClearQueue() - } - return nil } } diff --git a/internal/ui/model.go b/internal/ui/model.go index d1cbb143..d83471f1 100644 --- a/internal/ui/model.go +++ b/internal/ui/model.go @@ -2,6 +2,7 @@ package ui import ( "fmt" + "strings" "time" tea "charm.land/bubbletea/v2" @@ -28,14 +29,19 @@ const ( // makes the parent model easily testable with a mock. type AppController interface { // Run queues or immediately starts a new agent step with the given prompt. - // If an agent step is already in progress the prompt is queued and a - // QueueUpdatedEvent is sent to the program. - Run(prompt string) + // Returns the current queue depth: 0 means the prompt started immediately + // (or the app is closed), >0 means it was queued. The caller must update + // UI state (e.g. queueCount) based on the return value — Run does NOT + // send events to the program to avoid deadlocking when called from + // within Update(). + Run(prompt string) int // CancelCurrentStep cancels any in-progress agent step. CancelCurrentStep() // QueueLength returns the number of prompts currently waiting in the queue. QueueLength() int - // ClearQueue discards all queued prompts and emits a QueueUpdatedEvent. + // ClearQueue discards all queued prompts. The caller must update UI state + // (e.g. queueCount) — ClearQueue does NOT send events to the program to + // avoid deadlocking when called from within Update(). ClearQueue() // ClearMessages clears the conversation history. ClearMessages() @@ -83,7 +89,9 @@ type AppModelOptions struct { // // ┌─ stream region (variable height) ─────────────────┐ // │ │ -// ├─ separator line (with optional queue badge) ───────┤ +// ├─ separator line (with optional queue count) ───────┤ +// │ queued How do I fix the build? │ +// │ queued Also check the tests │ // └─ input region (fixed height from textarea) ────────┘ // // Completed responses are emitted above the BT-managed region via tea.Println() @@ -116,8 +124,10 @@ type AppModel struct { // modelName is the LLM model name shown in rendered messages. modelName string - // queueCount is cached from the last QueueUpdatedEvent for badge rendering. - queueCount int + // queuedMessages stores the text of prompts that were queued (not yet + // submitted to the agent). They are rendered with a "queued" badge above + // the input and move to scrollback when the agent picks them up. + queuedMessages []string // canceling tracks whether the user has pressed ESC once during stateWorking. // A second ESC within 2 seconds will cancel the current step. @@ -324,11 +334,24 @@ func (m *AppModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, tea.Batch(cmds...) } - // Regular prompt — print user message and forward to the app layer. - cmds = append(cmds, m.printUserMessage(msg.Text)) + // Regular prompt — forward to the app layer. if m.appCtrl != nil { - // app.Run() handles queueing internally if a step is in progress. - m.appCtrl.Run(msg.Text) + // Run returns the queue depth: >0 means the prompt was queued + // (agent is busy). We update queuedMessages directly here + // instead of relying on an event from prog.Send(), which would + // deadlock when called synchronously from within Update(). + if qLen := m.appCtrl.Run(msg.Text); qLen > 0 { + // Queued: anchor the message text above the input with a + // "queued" badge. It will be printed to scrollback when + // the agent picks it up (on QueueUpdatedEvent). + m.queuedMessages = append(m.queuedMessages, msg.Text) + m.distributeHeight() + } else { + // Started immediately: print to scrollback now. + cmds = append(cmds, m.printUserMessage(msg.Text)) + } + } else { + cmds = append(cmds, m.printUserMessage(msg.Text)) } if m.state != stateWorking { m.state = stateWorking @@ -398,7 +421,15 @@ func (m *AppModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // Informational — no action needed by parent. case app.QueueUpdatedEvent: - m.queueCount = msg.Length + // drainQueue popped item(s) from the queue. Move consumed messages + // from the anchored display to scrollback (they are now being processed + // or about to be). + for len(m.queuedMessages) > msg.Length { + text := m.queuedMessages[0] + m.queuedMessages = m.queuedMessages[1:] + cmds = append(cmds, m.printUserMessage(text)) + } + m.distributeHeight() case app.StepCompleteEvent: // Flush any remaining streamed text to scrollback, then reset stream @@ -438,17 +469,21 @@ func (m *AppModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } // View implements tea.Model. It renders the stacked layout: -// stream region + separator + input region. +// stream region + separator + [queued messages] + input region. func (m *AppModel) View() tea.View { streamView := m.renderStream() separator := m.renderSeparator() inputView := m.renderInput() - content := lipgloss.JoinVertical(lipgloss.Left, - streamView, - separator, - inputView, - ) + parts := []string{streamView, separator} + + if queuedView := m.renderQueuedMessages(); queuedView != "" { + parts = append(parts, queuedView) + } + + parts = append(parts, inputView) + + content := lipgloss.JoinVertical(lipgloss.Left, parts...) return tea.NewView(content) } @@ -478,15 +513,16 @@ func (m *AppModel) renderStream() string { return m.stream.View().Content } -// renderSeparator renders the separator line with an optional queue badge. +// renderSeparator renders the separator line with an optional queue count badge. func (m *AppModel) renderSeparator() string { theme := GetTheme() lineStyle := lipgloss.NewStyle().Foreground(theme.Muted) + queueLen := len(m.queuedMessages) - if m.queueCount > 0 { + if queueLen > 0 { badge := lipgloss.NewStyle(). Foreground(theme.Secondary). - Render(fmt.Sprintf("%d queued", m.queueCount)) + Render(fmt.Sprintf("%d queued", queueLen)) // Fill the separator with dashes up to the badge. dashWidth := m.width - lipgloss.Width(badge) - 1 @@ -508,6 +544,40 @@ func (m *AppModel) renderInput() string { return m.input.View().Content } +// renderQueuedMessages renders queued prompts with "queued" badges, anchored +// between the separator and input. Each message is shown on a single line +// with a styled badge so the user can see what is pending. +func (m *AppModel) renderQueuedMessages() string { + if len(m.queuedMessages) == 0 { + return "" + } + theme := GetTheme() + + badgeStyle := lipgloss.NewStyle(). + Foreground(theme.Secondary). + Bold(true) + badge := badgeStyle.Render("queued") + badgeWidth := lipgloss.Width(badge) + + textStyle := lipgloss.NewStyle().Foreground(theme.Muted) + // Indent to align with the input container (2-space left padding). + indent := " " + + var lines []string + for _, msg := range m.queuedMessages { + // Truncate long messages to fit on one line: + // indent(2) + badge + gap(2) + text must fit within m.width. + maxTextWidth := m.width - len(indent) - badgeWidth - 2 + display := msg + if maxTextWidth > 3 && len(display) > maxTextWidth { + display = display[:maxTextWidth-3] + "..." + } + line := indent + badge + " " + textStyle.Render(display) + lines = append(lines, line) + } + return strings.Join(lines, "\n") +} + // -------------------------------------------------------------------------- // Print helpers — emit content to scrollback via tea.Println // -------------------------------------------------------------------------- @@ -613,6 +683,8 @@ func (m *AppModel) handleSlashCommand(sc *SlashCommand) tea.Cmd { if m.appCtrl != nil { m.appCtrl.ClearQueue() } + m.queuedMessages = m.queuedMessages[:0] + m.distributeHeight() return nil default: return m.printSystemMessage(fmt.Sprintf("Unknown command: %s", sc.Name)) @@ -722,18 +794,21 @@ func (m *AppModel) flushStreamContent() tea.Cmd { } // distributeHeight recalculates child component heights after a window resize -// and propagates the computed stream height to the StreamComponent. +// or queue change, and propagates the computed stream height to the +// StreamComponent. // // Layout (line counts): // -// stream region = total - separator(1) - input(5) +// stream region = total - separator(1) - queued(N) - input(5) // separator = 1 line +// queued msgs = len(queuedMessages) lines (0 when queue is empty) // input region = 5 lines: title(1) + textarea(3) + help(1) func (m *AppModel) distributeHeight() { const separatorLines = 1 const inputLines = 5 // title (1) + textarea (3) + help (1) + queuedLines := len(m.queuedMessages) - streamHeight := m.height - separatorLines - inputLines + streamHeight := m.height - separatorLines - queuedLines - inputLines if streamHeight < 0 { streamHeight = 0 } diff --git a/internal/ui/model_test.go b/internal/ui/model_test.go index 219b28cf..4babc200 100644 --- a/internal/ui/model_test.go +++ b/internal/ui/model_test.go @@ -23,8 +23,9 @@ type stubAppController struct { queueLen int } -func (s *stubAppController) Run(prompt string) { +func (s *stubAppController) Run(prompt string) int { s.runCalls = append(s.runCalls, prompt) + return s.queueLen } func (s *stubAppController) CancelCurrentStep() { @@ -286,36 +287,63 @@ func TestESCCancel_clearedOnStepComplete(t *testing.T) { } // -------------------------------------------------------------------------- -// Queue badge update +// Queued messages // -------------------------------------------------------------------------- -// TestQueueBadge_updatesOnEvent verifies that QueueUpdatedEvent sets queueCount. -func TestQueueBadge_updatesOnEvent(t *testing.T) { - ctrl := &stubAppController{} +// TestQueuedMessages_storedOnQueuedSubmit verifies that submitting a prompt +// while the agent is busy stores the text in queuedMessages (not scrollback). +func TestQueuedMessages_storedOnQueuedSubmit(t *testing.T) { + ctrl := &stubAppController{queueLen: 1} // simulate busy (will return 1) m, _, _ := newTestAppModel(ctrl) + m.state = stateWorking - if m.queueCount != 0 { - t.Fatalf("expected queueCount=0 initially, got %d", m.queueCount) + _, cmd := m.Update(submitMsg{Text: "queued prompt"}) + + if len(m.queuedMessages) != 1 { + t.Fatalf("expected 1 queued message, got %d", len(m.queuedMessages)) } - - m = sendMsg(m, app.QueueUpdatedEvent{Length: 3}) - - if m.queueCount != 3 { - t.Fatalf("expected queueCount=3 after QueueUpdatedEvent, got %d", m.queueCount) + if m.queuedMessages[0] != "queued prompt" { + t.Fatalf("expected queued message text 'queued prompt', got %q", m.queuedMessages[0]) + } + // Should NOT produce a tea.Println cmd (message is anchored, not in scrollback). + if cmd != nil { + t.Fatal("expected nil cmd for queued submit (message should not print to scrollback)") } } -// TestQueueBadge_resetsToZero verifies that a QueueUpdatedEvent with Length=0 -// resets the badge count. -func TestQueueBadge_resetsToZero(t *testing.T) { +// TestQueuedMessages_poppedOnQueueUpdated verifies that QueueUpdatedEvent pops +// consumed messages from queuedMessages and prints them to scrollback. +func TestQueuedMessages_poppedOnQueueUpdated(t *testing.T) { ctrl := &stubAppController{} m, _, _ := newTestAppModel(ctrl) - m.queueCount = 5 + m.queuedMessages = []string{"first", "second", "third"} + + // Simulate drainQueue popping one item (length goes from 3 to 2). + _, cmd := m.Update(app.QueueUpdatedEvent{Length: 2}) + + if len(m.queuedMessages) != 2 { + t.Fatalf("expected 2 queued messages after pop, got %d", len(m.queuedMessages)) + } + if m.queuedMessages[0] != "second" { + t.Fatalf("expected first remaining message 'second', got %q", m.queuedMessages[0]) + } + // Should produce a cmd (tea.Println for the popped user message). + if cmd == nil { + t.Fatal("expected non-nil cmd (tea.Println) for popped message") + } +} + +// TestQueuedMessages_allPoppedOnDrain verifies that QueueUpdatedEvent with +// Length=0 pops all remaining queued messages. +func TestQueuedMessages_allPoppedOnDrain(t *testing.T) { + ctrl := &stubAppController{} + m, _, _ := newTestAppModel(ctrl) + m.queuedMessages = []string{"alpha", "beta"} m = sendMsg(m, app.QueueUpdatedEvent{Length: 0}) - if m.queueCount != 0 { - t.Fatalf("expected queueCount=0 after QueueUpdatedEvent{Length:0}, got %d", m.queueCount) + if len(m.queuedMessages) != 0 { + t.Fatalf("expected 0 queued messages after drain, got %d", len(m.queuedMessages)) } }