From 34591974a504e1df6441d83ca30ea1e9f6882e03 Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Wed, 22 Apr 2026 10:27:47 +0000 Subject: [PATCH 1/4] fix(cli): dispatch queued slash commands through the slash path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the agent was responding and the user queued a message, the drain path joined all queued messages with `\n\n` and submitted them as one prompt. Any slash command in that blob (e.g. `/model`) no longer started with `/`, so it was sent to the model as plain text instead of opening the command's dialog. The mid-turn tool-result drain had the same problem: it drained the entire queue into the tool-result payload, so a slash command queued during tool execution was injected as context for the model rather than executed as a command. Queue draining now splits into segments — consecutive plain-text messages are still batched into one submission, while slash commands are submitted alone so their `/` prefix survives. The mid-turn drain only takes leading plain-text messages and leaves slash commands queued for the normal idle drain. The idle drain is gated on open dialogs so a queued `/model` does not cause the following queued prompt to be sent to the model while the picker is still open, and a re-entry lock plus a nonce close the race between state commits and the async dialog-open. --- packages/cli/src/ui/AppContainer.test.tsx | 6 + packages/cli/src/ui/AppContainer.tsx | 59 ++- .../cli/src/ui/hooks/useMessageQueue.test.ts | 350 ++++++++++++------ packages/cli/src/ui/hooks/useMessageQueue.ts | 109 ++++-- 4 files changed, 368 insertions(+), 156 deletions(-) diff --git a/packages/cli/src/ui/AppContainer.test.tsx b/packages/cli/src/ui/AppContainer.test.tsx index 53da7cc32f0..4cb3f90e308 100644 --- a/packages/cli/src/ui/AppContainer.test.tsx +++ b/packages/cli/src/ui/AppContainer.test.tsx @@ -245,6 +245,7 @@ describe('AppContainer State Management', () => { getQueuedMessagesText: vi.fn().mockReturnValue(''), popAllMessages: vi.fn().mockReturnValue(null), drainQueue: vi.fn().mockReturnValue([]), + popNextSegment: vi.fn().mockReturnValue(null), }); mockedUseAutoAcceptIndicator.mockReturnValue(false); mockedUseGitBranchName.mockReturnValue('main'); @@ -459,6 +460,7 @@ describe('AppContainer State Management', () => { getQueuedMessagesText: vi.fn().mockReturnValue(''), popAllMessages: vi.fn().mockReturnValue(null), drainQueue: vi.fn().mockReturnValue([]), + popNextSegment: vi.fn().mockReturnValue(null), }); render( @@ -497,6 +499,7 @@ describe('AppContainer State Management', () => { getQueuedMessagesText: vi.fn().mockReturnValue(''), popAllMessages: vi.fn().mockReturnValue(null), drainQueue: vi.fn().mockReturnValue([]), + popNextSegment: vi.fn().mockReturnValue(null), }); render( @@ -577,6 +580,7 @@ describe('AppContainer State Management', () => { getQueuedMessagesText: vi.fn().mockReturnValue(''), popAllMessages: vi.fn().mockReturnValue(null), drainQueue: vi.fn().mockReturnValue([]), + popNextSegment: vi.fn().mockReturnValue(null), }); render( @@ -630,6 +634,7 @@ describe('AppContainer State Management', () => { getQueuedMessagesText: vi.fn().mockReturnValue('queued follow-up'), popAllMessages: mockPopAllMessages, drainQueue: vi.fn().mockReturnValue(['queued follow-up']), + popNextSegment: vi.fn().mockReturnValue('queued follow-up'), }); render( @@ -680,6 +685,7 @@ describe('AppContainer State Management', () => { getQueuedMessagesText: vi.fn().mockReturnValue('queued follow-up'), popAllMessages: vi.fn().mockReturnValue('queued follow-up'), drainQueue: vi.fn().mockReturnValue(['queued follow-up']), + popNextSegment: vi.fn().mockReturnValue('queued follow-up'), }); render( diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 89bb17befaf..5b93cc8ebe7 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -873,12 +873,17 @@ export const AppContainer = (props: AppContainerProps) => { disabled: agentViewState.activeView !== 'main', }); - const { messageQueue, addMessage, popAllMessages, drainQueue } = - useMessageQueue({ - isConfigInitialized, - streamingState, - submitQuery, - }); + const { + messageQueue, + addMessage, + popAllMessages, + drainQueue, + popNextSegment, + } = useMessageQueue({ + isConfigInitialized, + streamingState, + submitQuery, + }); // Bridge message queue to mid-turn drain via ref. // drainQueue reads the synchronous queueRef inside the hook, so it @@ -2029,6 +2034,48 @@ export const AppContainer = (props: AppContainerProps) => { isExtensionsManagerDialogOpen; dialogsVisibleRef.current = dialogsVisible; + // Drain the queued-messages queue when we're idle and not blocked by a + // dialog. One segment per effect run: consecutive plain-text messages are + // batched into a single submission, while each slash command is submitted + // alone so it's recognized as a command. When a slash command opens a + // dialog, `dialogsVisible` flips to true and subsequent segments stay + // queued until the dialog closes — avoids sending a queued prompt to the + // model while a picker is still open. + // + // `queueDrainingRef` blocks re-entry while an in-flight submission is + // still settling. Without it, calling `submitQuery` for a slash command + // returns control to React before the dialog-opening state flip commits; + // the effect would otherwise re-run on the fresh `messageQueue` value + // (post-pop) and drain the next segment before `dialogsVisible` has had a + // chance to flip true. `queueDrainNonce` is bumped once the submission + // settles so the effect re-fires for any remaining segments even when + // `streamingState` / `messageQueue` did not change (e.g. slash commands + // that don't start a model turn). + const queueDrainingRef = useRef(false); + const [queueDrainNonce, setQueueDrainNonce] = useState(0); + useEffect(() => { + if (queueDrainingRef.current) return; + if (!isConfigInitialized) return; + if (streamingState !== StreamingState.Idle) return; + if (dialogsVisible) return; + if (messageQueue.length === 0) return; + const segment = popNextSegment(); + if (segment === null) return; + queueDrainingRef.current = true; + Promise.resolve(submitQuery(segment)).finally(() => { + queueDrainingRef.current = false; + setQueueDrainNonce((n) => n + 1); + }); + }, [ + isConfigInitialized, + streamingState, + dialogsVisible, + messageQueue, + popNextSegment, + submitQuery, + queueDrainNonce, + ]); + const { isFeedbackDialogOpen, openFeedbackDialog, diff --git a/packages/cli/src/ui/hooks/useMessageQueue.test.ts b/packages/cli/src/ui/hooks/useMessageQueue.test.ts index 09cf4de2e53..0ddda6b01fb 100644 --- a/packages/cli/src/ui/hooks/useMessageQueue.test.ts +++ b/packages/cli/src/ui/hooks/useMessageQueue.test.ts @@ -119,120 +119,6 @@ describe('useMessageQueue', () => { ); }); - it('should auto-submit queued messages when transitioning to Idle', () => { - const { result, rerender } = renderHook( - ({ streamingState }) => - useMessageQueue({ - isConfigInitialized: true, - streamingState, - submitQuery: mockSubmitQuery, - }), - { - initialProps: { streamingState: StreamingState.Responding }, - }, - ); - - // Add some messages - act(() => { - result.current.addMessage('Message 1'); - result.current.addMessage('Message 2'); - }); - - expect(result.current.messageQueue).toEqual(['Message 1', 'Message 2']); - - // Transition to Idle - rerender({ streamingState: StreamingState.Idle }); - - expect(mockSubmitQuery).toHaveBeenCalledWith('Message 1\n\nMessage 2'); - expect(result.current.messageQueue).toEqual([]); - }); - - it('should not auto-submit when queue is empty', () => { - const { rerender } = renderHook( - ({ streamingState }) => - useMessageQueue({ - isConfigInitialized: true, - streamingState, - submitQuery: mockSubmitQuery, - }), - { - initialProps: { streamingState: StreamingState.Responding }, - }, - ); - - // Transition to Idle with empty queue - rerender({ streamingState: StreamingState.Idle }); - - expect(mockSubmitQuery).not.toHaveBeenCalled(); - }); - - it('should not auto-submit when not transitioning to Idle', () => { - const { result, rerender } = renderHook( - ({ streamingState }) => - useMessageQueue({ - isConfigInitialized: true, - streamingState, - submitQuery: mockSubmitQuery, - }), - { - initialProps: { streamingState: StreamingState.Responding }, - }, - ); - - // Add messages - act(() => { - result.current.addMessage('Message 1'); - }); - - // Transition to WaitingForConfirmation (not Idle) - rerender({ streamingState: StreamingState.WaitingForConfirmation }); - - expect(mockSubmitQuery).not.toHaveBeenCalled(); - expect(result.current.messageQueue).toEqual(['Message 1']); - }); - - it('should handle multiple state transitions correctly', () => { - const { result, rerender } = renderHook( - ({ streamingState }) => - useMessageQueue({ - isConfigInitialized: true, - streamingState, - submitQuery: mockSubmitQuery, - }), - { - initialProps: { streamingState: StreamingState.Idle }, - }, - ); - - // Start responding - rerender({ streamingState: StreamingState.Responding }); - - // Add messages while responding - act(() => { - result.current.addMessage('First batch'); - }); - - // Go back to idle - should submit - rerender({ streamingState: StreamingState.Idle }); - - expect(mockSubmitQuery).toHaveBeenCalledWith('First batch'); - expect(result.current.messageQueue).toEqual([]); - - // Start responding again - rerender({ streamingState: StreamingState.Responding }); - - // Add more messages - act(() => { - result.current.addMessage('Second batch'); - }); - - // Go back to idle - should submit again - rerender({ streamingState: StreamingState.Idle }); - - expect(mockSubmitQuery).toHaveBeenCalledWith('Second batch'); - expect(mockSubmitQuery).toHaveBeenCalledTimes(2); - }); - it('should pop all messages from queue', () => { const { result } = renderHook(() => useMessageQueue({ @@ -296,4 +182,240 @@ describe('useMessageQueue', () => { expect(popped).toBeNull(); expect(result.current.messageQueue).toEqual([]); }); + + describe('drainQueue (mid-turn drain for tool-result injection)', () => { + it('returns an empty array when the queue is empty', () => { + const { result } = renderHook(() => + useMessageQueue({ + isConfigInitialized: true, + streamingState: StreamingState.Responding, + submitQuery: mockSubmitQuery, + }), + ); + + let drained: string[] = []; + act(() => { + drained = result.current.drainQueue(); + }); + expect(drained).toEqual([]); + }); + + it('drains only leading plain-text messages and leaves slash commands queued', () => { + const { result } = renderHook(() => + useMessageQueue({ + isConfigInitialized: true, + streamingState: StreamingState.Responding, + submitQuery: mockSubmitQuery, + }), + ); + + act(() => { + result.current.addMessage('one'); + result.current.addMessage('two'); + result.current.addMessage('/model'); + result.current.addMessage('three'); + }); + + let drained: string[] = []; + act(() => { + drained = result.current.drainQueue(); + }); + + expect(drained).toEqual(['one', 'two']); + expect(result.current.messageQueue).toEqual(['/model', 'three']); + }); + + it('drains nothing when a slash command leads the queue', () => { + const { result } = renderHook(() => + useMessageQueue({ + isConfigInitialized: true, + streamingState: StreamingState.Responding, + submitQuery: mockSubmitQuery, + }), + ); + + act(() => { + result.current.addMessage('/model'); + result.current.addMessage('hello'); + }); + + let drained: string[] = []; + act(() => { + drained = result.current.drainQueue(); + }); + + expect(drained).toEqual([]); + expect(result.current.messageQueue).toEqual(['/model', 'hello']); + }); + + it('drains the whole queue when it contains no slash commands', () => { + const { result } = renderHook(() => + useMessageQueue({ + isConfigInitialized: true, + streamingState: StreamingState.Responding, + submitQuery: mockSubmitQuery, + }), + ); + + act(() => { + result.current.addMessage('a'); + result.current.addMessage('b'); + result.current.addMessage('c'); + }); + + let drained: string[] = []; + act(() => { + drained = result.current.drainQueue(); + }); + + expect(drained).toEqual(['a', 'b', 'c']); + expect(result.current.messageQueue).toEqual([]); + }); + }); + + describe('popNextSegment', () => { + it('returns null when the queue is empty', () => { + const { result } = renderHook(() => + useMessageQueue({ + isConfigInitialized: true, + streamingState: StreamingState.Idle, + submitQuery: mockSubmitQuery, + }), + ); + + let segment: string | null = null; + act(() => { + segment = result.current.popNextSegment(); + }); + expect(segment).toBeNull(); + }); + + it('batches leading plain-text messages into one segment', () => { + const { result } = renderHook(() => + useMessageQueue({ + isConfigInitialized: true, + streamingState: StreamingState.Responding, + submitQuery: mockSubmitQuery, + }), + ); + + act(() => { + result.current.addMessage('hello'); + result.current.addMessage('world'); + }); + + let segment: string | null = null; + act(() => { + segment = result.current.popNextSegment(); + }); + expect(segment).toBe('hello\n\nworld'); + expect(result.current.messageQueue).toEqual([]); + }); + + it('stops batching at the first slash command and leaves it queued', () => { + const { result } = renderHook(() => + useMessageQueue({ + isConfigInitialized: true, + streamingState: StreamingState.Responding, + submitQuery: mockSubmitQuery, + }), + ); + + act(() => { + result.current.addMessage('hello'); + result.current.addMessage('world'); + result.current.addMessage('/model'); + result.current.addMessage('after'); + }); + + let segment: string | null = null; + act(() => { + segment = result.current.popNextSegment(); + }); + expect(segment).toBe('hello\n\nworld'); + expect(result.current.messageQueue).toEqual(['/model', 'after']); + }); + + it('returns a slash command alone when it leads the queue', () => { + const { result } = renderHook(() => + useMessageQueue({ + isConfigInitialized: true, + streamingState: StreamingState.Responding, + submitQuery: mockSubmitQuery, + }), + ); + + act(() => { + result.current.addMessage('/model'); + result.current.addMessage('hello'); + }); + + let segment: string | null = null; + act(() => { + segment = result.current.popNextSegment(); + }); + expect(segment).toBe('/model'); + expect(result.current.messageQueue).toEqual(['hello']); + }); + + it('drains segments one at a time across repeated calls', () => { + const { result } = renderHook(() => + useMessageQueue({ + isConfigInitialized: true, + streamingState: StreamingState.Responding, + submitQuery: mockSubmitQuery, + }), + ); + + act(() => { + result.current.addMessage('hello'); + result.current.addMessage('/model'); + result.current.addMessage('world'); + }); + + const segments: Array = []; + act(() => { + segments.push(result.current.popNextSegment()); + }); + act(() => { + segments.push(result.current.popNextSegment()); + }); + act(() => { + segments.push(result.current.popNextSegment()); + }); + act(() => { + segments.push(result.current.popNextSegment()); + }); + + expect(segments).toEqual(['hello', '/model', 'world', null]); + expect(result.current.messageQueue).toEqual([]); + }); + + it('preserves remaining messages so popAllMessages can restore them after a cancel', () => { + const { result } = renderHook(() => + useMessageQueue({ + isConfigInitialized: true, + streamingState: StreamingState.Responding, + submitQuery: mockSubmitQuery, + }), + ); + + act(() => { + result.current.addMessage('hello'); + result.current.addMessage('/model'); + result.current.addMessage('after'); + }); + + act(() => { + result.current.popNextSegment(); + }); + expect(result.current.messageQueue).toEqual(['/model', 'after']); + + let popped: string | null = null; + act(() => { + popped = result.current.popAllMessages(); + }); + expect(popped).toBe('/model\n\nafter'); + }); + }); }); diff --git a/packages/cli/src/ui/hooks/useMessageQueue.ts b/packages/cli/src/ui/hooks/useMessageQueue.ts index cd1c8a294ea..f1474aed5c0 100644 --- a/packages/cli/src/ui/hooks/useMessageQueue.ts +++ b/packages/cli/src/ui/hooks/useMessageQueue.ts @@ -4,13 +4,39 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { useCallback, useEffect, useRef, useState } from 'react'; -import { StreamingState } from '../types.js'; +import { useCallback, useRef, useState } from 'react'; +import type { StreamingState } from '../types.js'; +import { isSlashCommand } from '../utils/commandUtils.js'; export interface UseMessageQueueOptions { isConfigInitialized: boolean; streamingState: StreamingState; - submitQuery: (query: string) => void; + submitQuery: (query: string) => void | Promise; +} + +// Extract the first submission segment from a queue. A segment is either a +// single slash-command message (submitted alone so `isSlashCommand` still +// fires at the receiver), or a batch of consecutive plain-text messages +// joined with `\n\n` (preserves the long-standing behavior where queued +// plain-text prompts are sent as one turn). The remaining messages stay in +// the queue; the next natural state transition drains them one segment at a +// time, which ensures a dialog-opening slash command (e.g. `/model`) does +// not auto-advance into subsequent messages while the dialog is still open. +function extractFirstSegment(messages: string[]): { + segment: string; + rest: string[]; +} { + if (isSlashCommand(messages[0])) { + return { segment: messages[0], rest: messages.slice(1) }; + } + const slashIdx = messages.findIndex((m) => isSlashCommand(m)); + if (slashIdx === -1) { + return { segment: messages.join('\n\n'), rest: [] }; + } + return { + segment: messages.slice(0, slashIdx).join('\n\n'), + rest: messages.slice(slashIdx), + }; } export interface UseMessageQueueReturn { @@ -20,11 +46,21 @@ export interface UseMessageQueueReturn { getQueuedMessagesText: () => string; popAllMessages: () => string | null; /** - * Atomically drain all queued messages. Returns the drained messages - * and clears both the synchronous ref and React state. Safe to call - * from non-React contexts (e.g., tool completion callbacks). + * Atomically drain leading plain-text messages from the queue, stopping + * at the first slash command. Returns the drained messages and updates + * both the synchronous ref and React state. Slash commands stay queued + * because they're UI actions, not model input — they should be executed + * via the normal idle drain, not injected as tool-result context. + * Safe to call from non-React contexts (e.g., tool completion callbacks). */ drainQueue: () => string[]; + /** + * Pop the next submission segment from the queue. Consecutive plain-text + * messages are batched into one segment joined by `\n\n`; slash commands + * are returned alone so the receiver's `isSlashCommand` check still fires. + * Returns `null` when the queue is empty. + */ + popNextSegment: () => string | null; } /** @@ -32,11 +68,9 @@ export interface UseMessageQueueReturn { * Allows users to queue messages while the AI is responding and automatically * sends them when streaming completes. */ -export function useMessageQueue({ - isConfigInitialized, - streamingState, - submitQuery, -}: UseMessageQueueOptions): UseMessageQueueReturn { +export function useMessageQueue( + _options: UseMessageQueueOptions, +): UseMessageQueueReturn { const [messageQueue, setMessageQueue] = useState([]); // Synchronous ref mirrors React state so non-React callbacks (e.g., // mid-turn drain in handleCompletedTools) always see the latest queue. @@ -74,35 +108,37 @@ export function useMessageQueue({ return allText; }, []); - // Atomically drain all queued messages (synchronous, safe from callbacks). + // Atomically drain leading plain-text messages (synchronous, safe from + // callbacks). Stops at the first slash command so those stay queued and + // get dispatched through the normal idle-drain path instead of being + // injected into the model's context as raw text. const drainQueue = useCallback((): string[] => { - const drained = queueRef.current; - if (drained.length === 0) return []; - queueRef.current = []; - setMessageQueue([]); + const current = queueRef.current; + if (current.length === 0) return []; + if (isSlashCommand(current[0])) return []; + const slashIdx = current.findIndex((m) => isSlashCommand(m)); + if (slashIdx === -1) { + queueRef.current = []; + setMessageQueue([]); + return current; + } + const drained = current.slice(0, slashIdx); + const rest = current.slice(slashIdx); + queueRef.current = rest; + setMessageQueue(rest); return drained; }, []); - // Process queued messages when streaming becomes idle - useEffect(() => { - if ( - isConfigInitialized && - streamingState === StreamingState.Idle && - messageQueue.length > 0 - ) { - // Combine all messages with double newlines for clarity - const combinedMessage = messageQueue.join('\n\n'); - // Clear the queue and submit - clearQueue(); - submitQuery(combinedMessage); - } - }, [ - isConfigInitialized, - streamingState, - messageQueue, - submitQuery, - clearQueue, - ]); + // Pop the next submission segment. Caller is responsible for gating on + // streamingState, open dialogs, etc.; this hook only owns queue state. + const popNextSegment = useCallback((): string | null => { + const current = queueRef.current; + if (current.length === 0) return null; + const { segment, rest } = extractFirstSegment(current); + queueRef.current = rest; + setMessageQueue(rest); + return segment; + }, []); return { messageQueue, @@ -111,5 +147,6 @@ export function useMessageQueue({ getQueuedMessagesText, popAllMessages, drainQueue, + popNextSegment, }; } From 15f89931e8c51ab2cc391215e668b2aff83755c2 Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Thu, 23 Apr 2026 17:27:13 +0800 Subject: [PATCH 2/4] fix(cli): defer queued slash commands until idle --- packages/cli/src/ui/AppContainer.test.tsx | 38 ++++++++++++++ packages/cli/src/ui/AppContainer.tsx | 31 ++++++------ .../cli/src/ui/hooks/useMessageQueue.test.ts | 15 +++--- packages/cli/src/ui/hooks/useMessageQueue.ts | 50 +++++++------------ 4 files changed, 80 insertions(+), 54 deletions(-) diff --git a/packages/cli/src/ui/AppContainer.test.tsx b/packages/cli/src/ui/AppContainer.test.tsx index 4cb3f90e308..f7e63b3920e 100644 --- a/packages/cli/src/ui/AppContainer.test.tsx +++ b/packages/cli/src/ui/AppContainer.test.tsx @@ -478,6 +478,44 @@ describe('AppContainer State Management', () => { expect(mockQueueMessage).not.toHaveBeenCalled(); }); + it('submits slash commands immediately instead of queueing while idle', () => { + const mockSubmitQuery = vi.fn(); + const mockQueueMessage = vi.fn(); + + mockedUseGeminiStream.mockReturnValue({ + streamingState: 'idle', + submitQuery: mockSubmitQuery, + initError: null, + pendingHistoryItems: [], + thought: null, + cancelOngoingRequest: vi.fn(), + retryLastPrompt: vi.fn(), + }); + mockedUseMessageQueue.mockReturnValue({ + messageQueue: [], + addMessage: mockQueueMessage, + clearQueue: vi.fn(), + getQueuedMessagesText: vi.fn().mockReturnValue(''), + popAllMessages: vi.fn().mockReturnValue(null), + drainQueue: vi.fn().mockReturnValue([]), + popNextSegment: vi.fn().mockReturnValue(null), + }); + + render( + , + ); + + capturedUIActions.handleFinalSubmit('/model'); + + expect(mockSubmitQuery).toHaveBeenCalledWith('/model'); + expect(mockQueueMessage).not.toHaveBeenCalled(); + }); + it.each(['exit', 'quit', ':q', ':q!', ':wq', ':wq!'])( 'routes bare "%s" to /quit instead of sending as a message', (command) => { diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 5b93cc8ebe7..526722128df 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -88,7 +88,7 @@ import { useTextBuffer } from './components/shared/text-buffer.js'; import { useLogger } from './hooks/useLogger.js'; import { useGeminiStream } from './hooks/useGeminiStream.js'; import { useVim } from './hooks/vim.js'; -import { isBtwCommand } from './utils/commandUtils.js'; +import { isBtwCommand, isSlashCommand } from './utils/commandUtils.js'; import { type LoadedSettings, SettingScope } from '../config/settings.js'; import { type InitializationResult } from '../core/initializer.js'; import { useFocus } from './hooks/useFocus.js'; @@ -1163,6 +1163,14 @@ export const AppContainer = (props: AppContainerProps) => { speculationRef.current = IDLE_SPECULATION; } + if ( + streamingState === StreamingState.Idle && + isSlashCommand(submittedValue) + ) { + void submitQuery(submittedValue); + return; + } + addMessage(submittedValue); }, [ @@ -1218,8 +1226,8 @@ export const AppContainer = (props: AppContainerProps) => { // particular, do NOT repopulate it with the previous prompt; the user // can still recall it via history navigation (Up/Ctrl+P). // - // popAllMessages is atomic via the queue's synchronous ref, matching - // the drain behavior used during tool completion. + // popAllMessages is atomic via the queue's synchronous ref, + // matching the drain behavior used during tool completion. const popped = popAllMessages(); if (popped) { const currentText = buffer.text; @@ -2036,21 +2044,14 @@ export const AppContainer = (props: AppContainerProps) => { // Drain the queued-messages queue when we're idle and not blocked by a // dialog. One segment per effect run: consecutive plain-text messages are - // batched into a single submission, while each slash command is submitted - // alone so it's recognized as a command. When a slash command opens a - // dialog, `dialogsVisible` flips to true and subsequent segments stay - // queued until the dialog closes — avoids sending a queued prompt to the - // model while a picker is still open. + // batched into a single submission, while queued slash commands are + // submitted alone once the app is idle again. Mid-turn drain still keeps + // slash commands out of tool-result injection. // // `queueDrainingRef` blocks re-entry while an in-flight submission is - // still settling. Without it, calling `submitQuery` for a slash command - // returns control to React before the dialog-opening state flip commits; - // the effect would otherwise re-run on the fresh `messageQueue` value - // (post-pop) and drain the next segment before `dialogsVisible` has had a - // chance to flip true. `queueDrainNonce` is bumped once the submission + // still settling. `queueDrainNonce` is bumped once the submission // settles so the effect re-fires for any remaining segments even when - // `streamingState` / `messageQueue` did not change (e.g. slash commands - // that don't start a model turn). + // `streamingState` / `messageQueue` did not change. const queueDrainingRef = useRef(false); const [queueDrainNonce, setQueueDrainNonce] = useState(0); useEffect(() => { diff --git a/packages/cli/src/ui/hooks/useMessageQueue.test.ts b/packages/cli/src/ui/hooks/useMessageQueue.test.ts index 0ddda6b01fb..6452441f2a0 100644 --- a/packages/cli/src/ui/hooks/useMessageQueue.test.ts +++ b/packages/cli/src/ui/hooks/useMessageQueue.test.ts @@ -200,7 +200,7 @@ describe('useMessageQueue', () => { expect(drained).toEqual([]); }); - it('drains only leading plain-text messages and leaves slash commands queued', () => { + it('drains all plain-text messages and leaves slash commands queued', () => { const { result } = renderHook(() => useMessageQueue({ isConfigInitialized: true, @@ -221,11 +221,11 @@ describe('useMessageQueue', () => { drained = result.current.drainQueue(); }); - expect(drained).toEqual(['one', 'two']); - expect(result.current.messageQueue).toEqual(['/model', 'three']); + expect(drained).toEqual(['one', 'two', 'three']); + expect(result.current.messageQueue).toEqual(['/model']); }); - it('drains nothing when a slash command leads the queue', () => { + it('still drains later plain-text messages when a slash command leads the queue', () => { const { result } = renderHook(() => useMessageQueue({ isConfigInitialized: true, @@ -244,8 +244,8 @@ describe('useMessageQueue', () => { drained = result.current.drainQueue(); }); - expect(drained).toEqual([]); - expect(result.current.messageQueue).toEqual(['/model', 'hello']); + expect(drained).toEqual(['hello']); + expect(result.current.messageQueue).toEqual(['/model']); }); it('drains the whole queue when it contains no slash commands', () => { @@ -415,7 +415,8 @@ describe('useMessageQueue', () => { act(() => { popped = result.current.popAllMessages(); }); - expect(popped).toBe('/model\n\nafter'); + expect(popped).toBe('/model'); + expect(result.current.messageQueue).toEqual(['after']); }); }); }); diff --git a/packages/cli/src/ui/hooks/useMessageQueue.ts b/packages/cli/src/ui/hooks/useMessageQueue.ts index f1474aed5c0..944fb39af50 100644 --- a/packages/cli/src/ui/hooks/useMessageQueue.ts +++ b/packages/cli/src/ui/hooks/useMessageQueue.ts @@ -14,14 +14,9 @@ export interface UseMessageQueueOptions { submitQuery: (query: string) => void | Promise; } -// Extract the first submission segment from a queue. A segment is either a -// single slash-command message (submitted alone so `isSlashCommand` still -// fires at the receiver), or a batch of consecutive plain-text messages -// joined with `\n\n` (preserves the long-standing behavior where queued -// plain-text prompts are sent as one turn). The remaining messages stay in -// the queue; the next natural state transition drains them one segment at a -// time, which ensures a dialog-opening slash command (e.g. `/model`) does -// not auto-advance into subsequent messages while the dialog is still open. +// Extract the first queue segment in original order. A segment is either a +// single slash command or a batch of consecutive plain-text prompts joined +// with `\n\n`. function extractFirstSegment(messages: string[]): { segment: string; rest: string[]; @@ -46,11 +41,10 @@ export interface UseMessageQueueReturn { getQueuedMessagesText: () => string; popAllMessages: () => string | null; /** - * Atomically drain leading plain-text messages from the queue, stopping - * at the first slash command. Returns the drained messages and updates - * both the synchronous ref and React state. Slash commands stay queued - * because they're UI actions, not model input — they should be executed - * via the normal idle drain, not injected as tool-result context. + * Atomically drain every queued plain-text prompt from the queue while + * leaving slash commands deferred for manual execution. Returns the + * drained prompts in their original order and updates both the + * synchronous ref and React state. * Safe to call from non-React contexts (e.g., tool completion callbacks). */ drainQueue: () => string[]; @@ -97,33 +91,25 @@ export function useMessageQueue( return messageQueue.join('\n\n'); }, [messageQueue]); - // Pop all messages from the queue for editing (atomic via ref to prevent - // duplicate pops from key auto-repeat before React re-renders) + // Pop deferred queue content for editing (atomic via ref to prevent + // duplicate pops from key auto-repeat before React re-renders). const popAllMessages = useCallback((): string | null => { const current = queueRef.current; if (current.length === 0) return null; - const allText = current.join('\n\n'); - queueRef.current = []; - setMessageQueue([]); - return allText; + const { segment, rest } = extractFirstSegment(current); + queueRef.current = rest; + setMessageQueue(rest); + return segment; }, []); - // Atomically drain leading plain-text messages (synchronous, safe from - // callbacks). Stops at the first slash command so those stay queued and - // get dispatched through the normal idle-drain path instead of being - // injected into the model's context as raw text. + // Atomically drain every plain-text prompt (synchronous, safe from + // callbacks) while leaving slash commands deferred for manual execution. const drainQueue = useCallback((): string[] => { const current = queueRef.current; if (current.length === 0) return []; - if (isSlashCommand(current[0])) return []; - const slashIdx = current.findIndex((m) => isSlashCommand(m)); - if (slashIdx === -1) { - queueRef.current = []; - setMessageQueue([]); - return current; - } - const drained = current.slice(0, slashIdx); - const rest = current.slice(slashIdx); + const drained = current.filter((message) => !isSlashCommand(message)); + if (drained.length === 0) return []; + const rest = current.filter((message) => isSlashCommand(message)); queueRef.current = rest; setMessageQueue(rest); return drained; From b62ac732543f98f8fa698907ac0cb2c0996ae450 Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Fri, 24 Apr 2026 12:18:56 +0800 Subject: [PATCH 3/4] fix(cli): drop queued messages on cancel instead of auto-submitting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cancel's contract is now "abort and redirect" in both cancel paths: restore the most recent queued segment into the buffer for editing and drop the rest, so forgotten follow-ups cannot auto-submit once the turn settles. Previously the non-tool path left queued plain-text segments in place for the idle drain to fire, and the tool-executing path cleared only the buffer — both surprised users with belated message dispatches after they had already cancelled. --- packages/cli/src/ui/AppContainer.test.tsx | 73 ++++++++++++++++++++++- packages/cli/src/ui/AppContainer.tsx | 23 ++++--- 2 files changed, 87 insertions(+), 9 deletions(-) diff --git a/packages/cli/src/ui/AppContainer.test.tsx b/packages/cli/src/ui/AppContainer.test.tsx index f7e63b3920e..c5eb7c0434a 100644 --- a/packages/cli/src/ui/AppContainer.test.tsx +++ b/packages/cli/src/ui/AppContainer.test.tsx @@ -28,6 +28,7 @@ import { UIActionsContext, type UIActions, } from './contexts/UIActionsContext.js'; +import { ToolCallStatus } from './types.js'; import { useContext } from 'react'; // Mock useStdout to capture terminal title writes @@ -647,6 +648,7 @@ describe('AppContainer State Management', () => { it('moves queued follow-up messages into an empty buffer on cancel', async () => { const mockSetText = vi.fn(); const mockPopAllMessages = vi.fn().mockReturnValue('queued follow-up'); + const mockClearQueue = vi.fn(); mockedUseTextBuffer.mockReturnValue({ text: '', setText: mockSetText, @@ -668,7 +670,7 @@ describe('AppContainer State Management', () => { mockedUseMessageQueue.mockReturnValue({ messageQueue: ['queued follow-up'], addMessage: vi.fn(), - clearQueue: vi.fn(), + clearQueue: mockClearQueue, getQueuedMessagesText: vi.fn().mockReturnValue('queued follow-up'), popAllMessages: mockPopAllMessages, drainQueue: vi.fn().mockReturnValue(['queued follow-up']), @@ -696,6 +698,75 @@ describe('AppContainer State Management', () => { expect.stringContaining('the previous prompt'), ); expect(mockPopAllMessages).toHaveBeenCalled(); + // Option C: cancel restores the first segment and drops the rest of + // the queue so forgotten follow-ups never auto-submit later. + expect(mockClearQueue).toHaveBeenCalled(); + }); + + it('drops the queue when cancelling during tool execution', async () => { + // Simulates: user asks for a shell tool (e.g. sleep 30), queues + // `/model` and `hi` while the tool is running, then hits Ctrl+C. + // The cancel must clear BOTH the buffer and the queue so that + // `hi` does not auto-fire once the tool settles and the app + // returns to idle. + const mockSetText = vi.fn(); + const mockClearQueue = vi.fn(); + mockedUseTextBuffer.mockReturnValue({ + text: '', + setText: mockSetText, + }); + installCancelCapture({ + streamingState: 'responding', + submitQuery: vi.fn(), + initError: null, + pendingHistoryItems: [ + { + type: 'tool_group', + tools: [ + { + callId: 'call-1', + name: 'run_shell_command', + description: 'sleep 30', + status: ToolCallStatus.Executing, + resultDisplay: undefined, + confirmationDetails: undefined, + renderOutputAsMarkdown: false, + }, + ], + }, + ], + thought: null, + cancelOngoingRequest: vi.fn(), + retryLastPrompt: vi.fn(), + }); + mockedUseMessageQueue.mockReturnValue({ + messageQueue: ['/model', 'hi'], + addMessage: vi.fn(), + clearQueue: mockClearQueue, + getQueuedMessagesText: vi.fn().mockReturnValue('/model\n\nhi'), + popAllMessages: vi.fn().mockReturnValue('/model'), + drainQueue: vi.fn().mockReturnValue([]), + popNextSegment: vi.fn().mockReturnValue('/model'), + }); + + render( + , + ); + + await Promise.resolve(); + await Promise.resolve(); + + triggerCancel(); + + // Buffer cleared and queue dropped — same "abort and redirect" + // contract as the non-tool cancel path. + expect(mockSetText).toHaveBeenCalledWith(''); + expect(mockClearQueue).toHaveBeenCalled(); }); it('preserves an in-progress draft when restoring queued messages on cancel', async () => { diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 526722128df..e8d0101d183 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -876,6 +876,7 @@ export const AppContainer = (props: AppContainerProps) => { const { messageQueue, addMessage, + clearQueue, popAllMessages, drainQueue, popNextSegment, @@ -1217,19 +1218,24 @@ export const AppContainer = (props: AppContainerProps) => { ...pendingGeminiHistoryItems, ]; if (isToolExecuting(pendingHistoryItems)) { - buffer.setText(''); // Just clear the prompt + // Cancel during tool execution: clear the buffer AND drop any + // queued follow-ups. The cancel contract is the same as the + // non-tool path — abort and redirect — so queued segments must + // not auto-fire once the tool settles and the app returns to idle. + buffer.setText(''); + clearQueue(); return; } - // Move any queued follow-up messages back into the buffer so the user - // can edit or resubmit them. Otherwise leave the buffer alone — in - // particular, do NOT repopulate it with the previous prompt; the user - // can still recall it via history navigation (Up/Ctrl+P). - // - // popAllMessages is atomic via the queue's synchronous ref, - // matching the drain behavior used during tool completion. + // Cancel is "abort and redirect": restore the most recent queued + // segment into the buffer for editing, and discard the rest of the + // queue so the user is not surprised later by auto-submission of + // forgotten follow-ups. Segment boundaries still matter for the + // normal idle drain, which is the primary consumer of the queue; + // on cancel, the user's intent is a clean slate. const popped = popAllMessages(); if (popped) { + clearQueue(); const currentText = buffer.text; // Preserve any in-progress draft the user typed since submitting (this // is reachable via Ctrl+C cancel, which fires regardless of buffer @@ -1239,6 +1245,7 @@ export const AppContainer = (props: AppContainerProps) => { }, [ buffer, popAllMessages, + clearQueue, pendingSlashCommandHistoryItems, pendingGeminiHistoryItems, ]); From 00187be0596287696959da3b64f2d5662f573034 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=84=9A=E8=BF=9C?= Date: Fri, 24 Apr 2026 15:43:21 +0800 Subject: [PATCH 4/4] refactor(cli): batch plain prompts in idle drain Idle drain now runs in two phases: drain all plain-text prompts into one turn (drainQueue), then pop slash commands one-by-one (popNextSegment). Mirrors the mid-turn behavior so queue handling is consistent across mid-turn and idle contexts. popAllMessages now drains the entire queue joined with \n\n for Ctrl+C cancel and ESC/Up edit-restore. Drop the unused options parameter from useMessageQueue and the extractFirstSegment helper. --- packages/cli/src/ui/AppContainer.test.tsx | 6 +- packages/cli/src/ui/AppContainer.tsx | 48 +-- .../cli/src/ui/hooks/useMessageQueue.test.ts | 299 +++++------------- packages/cli/src/ui/hooks/useMessageQueue.ts | 75 +---- 4 files changed, 105 insertions(+), 323 deletions(-) diff --git a/packages/cli/src/ui/AppContainer.test.tsx b/packages/cli/src/ui/AppContainer.test.tsx index c5eb7c0434a..5e473f2080c 100644 --- a/packages/cli/src/ui/AppContainer.test.tsx +++ b/packages/cli/src/ui/AppContainer.test.tsx @@ -698,9 +698,9 @@ describe('AppContainer State Management', () => { expect.stringContaining('the previous prompt'), ); expect(mockPopAllMessages).toHaveBeenCalled(); - // Option C: cancel restores the first segment and drops the rest of - // the queue so forgotten follow-ups never auto-submit later. - expect(mockClearQueue).toHaveBeenCalled(); + // popAllForEdit drains the queue internally, so the cancel handler + // does not need to call clearQueue separately on this path. + expect(mockClearQueue).not.toHaveBeenCalled(); }); it('drops the queue when cancelling during tool execution', async () => { diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index e8d0101d183..d2f81a67412 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -880,15 +880,11 @@ export const AppContainer = (props: AppContainerProps) => { popAllMessages, drainQueue, popNextSegment, - } = useMessageQueue({ - isConfigInitialized, - streamingState, - submitQuery, - }); + } = useMessageQueue(); // Bridge message queue to mid-turn drain via ref. // drainQueue reads the synchronous queueRef inside the hook, so it - // stays consistent with popAllMessages even before React re-renders. + // stays consistent with popNextSegment even before React re-renders. midTurnDrainRef.current = drainQueue; // Connect remote input watcher to submitQuery for bidirectional sync. @@ -1218,28 +1214,16 @@ export const AppContainer = (props: AppContainerProps) => { ...pendingGeminiHistoryItems, ]; if (isToolExecuting(pendingHistoryItems)) { - // Cancel during tool execution: clear the buffer AND drop any - // queued follow-ups. The cancel contract is the same as the - // non-tool path — abort and redirect — so queued segments must - // not auto-fire once the tool settles and the app returns to idle. + // Tool-cancel: drop both buffer and queue so nothing auto-fires later. buffer.setText(''); clearQueue(); return; } - // Cancel is "abort and redirect": restore the most recent queued - // segment into the buffer for editing, and discard the rest of the - // queue so the user is not surprised later by auto-submission of - // forgotten follow-ups. Segment boundaries still matter for the - // normal idle drain, which is the primary consumer of the queue; - // on cancel, the user's intent is a clean slate. + // Restore queued input joined into the buffer for editing. const popped = popAllMessages(); if (popped) { - clearQueue(); const currentText = buffer.text; - // Preserve any in-progress draft the user typed since submitting (this - // is reachable via Ctrl+C cancel, which fires regardless of buffer - // content). Mirrors the popQueueIntoInput convention in InputPrompt. buffer.setText(currentText ? `${popped}\n${currentText}` : popped); } }, [ @@ -2049,16 +2033,8 @@ export const AppContainer = (props: AppContainerProps) => { isExtensionsManagerDialogOpen; dialogsVisibleRef.current = dialogsVisible; - // Drain the queued-messages queue when we're idle and not blocked by a - // dialog. One segment per effect run: consecutive plain-text messages are - // batched into a single submission, while queued slash commands are - // submitted alone once the app is idle again. Mid-turn drain still keeps - // slash commands out of tool-result injection. - // - // `queueDrainingRef` blocks re-entry while an in-flight submission is - // still settling. `queueDrainNonce` is bumped once the submission - // settles so the effect re-fires for any remaining segments even when - // `streamingState` / `messageQueue` did not change. + // Drain queued messages when idle. `queueDrainNonce` re-fires the effect + // after each submission settles so multi-step queues drain end-to-end. const queueDrainingRef = useRef(false); const [queueDrainNonce, setQueueDrainNonce] = useState(0); useEffect(() => { @@ -2067,10 +2043,15 @@ export const AppContainer = (props: AppContainerProps) => { if (streamingState !== StreamingState.Idle) return; if (dialogsVisible) return; if (messageQueue.length === 0) return; - const segment = popNextSegment(); - if (segment === null) return; + + // Two-phase: batch plain prompts as one turn, else pop next slash command. + const plainPrompts = drainQueue(); + const submission = + plainPrompts.length > 0 ? plainPrompts.join('\n\n') : popNextSegment(); + if (submission === null) return; + queueDrainingRef.current = true; - Promise.resolve(submitQuery(segment)).finally(() => { + Promise.resolve(submitQuery(submission)).finally(() => { queueDrainingRef.current = false; setQueueDrainNonce((n) => n + 1); }); @@ -2079,6 +2060,7 @@ export const AppContainer = (props: AppContainerProps) => { streamingState, dialogsVisible, messageQueue, + drainQueue, popNextSegment, submitQuery, queueDrainNonce, diff --git a/packages/cli/src/ui/hooks/useMessageQueue.test.ts b/packages/cli/src/ui/hooks/useMessageQueue.test.ts index 6452441f2a0..6d783d0d81e 100644 --- a/packages/cli/src/ui/hooks/useMessageQueue.test.ts +++ b/packages/cli/src/ui/hooks/useMessageQueue.test.ts @@ -7,13 +7,9 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { renderHook, act } from '@testing-library/react'; import { useMessageQueue } from './useMessageQueue.js'; -import { StreamingState } from '../types.js'; describe('useMessageQueue', () => { - let mockSubmitQuery: ReturnType; - beforeEach(() => { - mockSubmitQuery = vi.fn(); vi.useFakeTimers(); }); @@ -23,26 +19,14 @@ describe('useMessageQueue', () => { }); it('should initialize with empty queue', () => { - const { result } = renderHook(() => - useMessageQueue({ - isConfigInitialized: true, - streamingState: StreamingState.Idle, - submitQuery: mockSubmitQuery, - }), - ); + const { result } = renderHook(() => useMessageQueue()); expect(result.current.messageQueue).toEqual([]); expect(result.current.getQueuedMessagesText()).toBe(''); }); it('should add messages to queue', () => { - const { result } = renderHook(() => - useMessageQueue({ - isConfigInitialized: true, - streamingState: StreamingState.Responding, - submitQuery: mockSubmitQuery, - }), - ); + const { result } = renderHook(() => useMessageQueue()); act(() => { result.current.addMessage('Test message 1'); @@ -56,13 +40,7 @@ describe('useMessageQueue', () => { }); it('should filter out empty messages', () => { - const { result } = renderHook(() => - useMessageQueue({ - isConfigInitialized: true, - streamingState: StreamingState.Responding, - submitQuery: mockSubmitQuery, - }), - ); + const { result } = renderHook(() => useMessageQueue()); act(() => { result.current.addMessage('Valid message'); @@ -78,13 +56,7 @@ describe('useMessageQueue', () => { }); it('should clear queue', () => { - const { result } = renderHook(() => - useMessageQueue({ - isConfigInitialized: true, - streamingState: StreamingState.Responding, - submitQuery: mockSubmitQuery, - }), - ); + const { result } = renderHook(() => useMessageQueue()); act(() => { result.current.addMessage('Test message'); @@ -100,13 +72,7 @@ describe('useMessageQueue', () => { }); it('should return queued messages as text with double newlines', () => { - const { result } = renderHook(() => - useMessageQueue({ - isConfigInitialized: true, - streamingState: StreamingState.Responding, - submitQuery: mockSubmitQuery, - }), - ); + const { result } = renderHook(() => useMessageQueue()); act(() => { result.current.addMessage('Message 1'); @@ -119,79 +85,78 @@ describe('useMessageQueue', () => { ); }); - it('should pop all messages from queue', () => { - const { result } = renderHook(() => - useMessageQueue({ - isConfigInitialized: true, - streamingState: StreamingState.Responding, - submitQuery: mockSubmitQuery, - }), - ); + describe('popAllMessages (cancel and ESC/Up restore)', () => { + it('returns null when the queue is empty', () => { + const { result } = renderHook(() => useMessageQueue()); - act(() => { - result.current.addMessage('Message 1'); - result.current.addMessage('Message 2'); - result.current.addMessage('Message 3'); - }); + let popped: string | null = null; + act(() => { + popped = result.current.popAllMessages(); + }); - let popped: string | null = null; - act(() => { - popped = result.current.popAllMessages(); + expect(popped).toBeNull(); + expect(result.current.messageQueue).toEqual([]); }); - expect(popped).toBe('Message 1\n\nMessage 2\n\nMessage 3'); - expect(result.current.messageQueue).toEqual([]); - }); + it('joins all queued messages with double newlines and clears the queue', () => { + const { result } = renderHook(() => useMessageQueue()); - it('should pop single message without separator', () => { - const { result } = renderHook(() => - useMessageQueue({ - isConfigInitialized: true, - streamingState: StreamingState.Responding, - submitQuery: mockSubmitQuery, - }), - ); + act(() => { + result.current.addMessage('Message 1'); + result.current.addMessage('Message 2'); + result.current.addMessage('Message 3'); + }); - act(() => { - result.current.addMessage('Only message'); - }); + let popped: string | null = null; + act(() => { + popped = result.current.popAllMessages(); + }); - let popped: string | null = null; - act(() => { - popped = result.current.popAllMessages(); + expect(popped).toBe('Message 1\n\nMessage 2\n\nMessage 3'); + expect(result.current.messageQueue).toEqual([]); }); - expect(popped).toBe('Only message'); - expect(result.current.messageQueue).toEqual([]); - }); + it('returns a single message without separator', () => { + const { result } = renderHook(() => useMessageQueue()); - it('should return null when popping from empty queue', () => { - const { result } = renderHook(() => - useMessageQueue({ - isConfigInitialized: true, - streamingState: StreamingState.Responding, - submitQuery: mockSubmitQuery, - }), - ); + act(() => { + result.current.addMessage('Only message'); + }); - let popped: string | null = null; - act(() => { - popped = result.current.popAllMessages(); + let popped: string | null = null; + act(() => { + popped = result.current.popAllMessages(); + }); + + expect(popped).toBe('Only message'); + expect(result.current.messageQueue).toEqual([]); }); - expect(popped).toBeNull(); - expect(result.current.messageQueue).toEqual([]); + it('joins mixed slash commands and prompts in original order', () => { + // Edit-restore intentionally collapses segment boundaries: the user is + // recovering input into the buffer to edit before resubmitting, so + // typing order matters more than slash-vs-prompt routing boundaries. + const { result } = renderHook(() => useMessageQueue()); + + act(() => { + result.current.addMessage('/model'); + result.current.addMessage('hello'); + result.current.addMessage('world'); + }); + + let popped: string | null = null; + act(() => { + popped = result.current.popAllMessages(); + }); + + expect(popped).toBe('/model\n\nhello\n\nworld'); + expect(result.current.messageQueue).toEqual([]); + }); }); describe('drainQueue (mid-turn drain for tool-result injection)', () => { it('returns an empty array when the queue is empty', () => { - const { result } = renderHook(() => - useMessageQueue({ - isConfigInitialized: true, - streamingState: StreamingState.Responding, - submitQuery: mockSubmitQuery, - }), - ); + const { result } = renderHook(() => useMessageQueue()); let drained: string[] = []; act(() => { @@ -201,13 +166,7 @@ describe('useMessageQueue', () => { }); it('drains all plain-text messages and leaves slash commands queued', () => { - const { result } = renderHook(() => - useMessageQueue({ - isConfigInitialized: true, - streamingState: StreamingState.Responding, - submitQuery: mockSubmitQuery, - }), - ); + const { result } = renderHook(() => useMessageQueue()); act(() => { result.current.addMessage('one'); @@ -225,18 +184,12 @@ describe('useMessageQueue', () => { expect(result.current.messageQueue).toEqual(['/model']); }); - it('still drains later plain-text messages when a slash command leads the queue', () => { - const { result } = renderHook(() => - useMessageQueue({ - isConfigInitialized: true, - streamingState: StreamingState.Responding, - submitQuery: mockSubmitQuery, - }), - ); + it('returns an empty array when the queue contains only slash commands', () => { + const { result } = renderHook(() => useMessageQueue()); act(() => { result.current.addMessage('/model'); - result.current.addMessage('hello'); + result.current.addMessage('/help'); }); let drained: string[] = []; @@ -244,18 +197,12 @@ describe('useMessageQueue', () => { drained = result.current.drainQueue(); }); - expect(drained).toEqual(['hello']); - expect(result.current.messageQueue).toEqual(['/model']); + expect(drained).toEqual([]); + expect(result.current.messageQueue).toEqual(['/model', '/help']); }); it('drains the whole queue when it contains no slash commands', () => { - const { result } = renderHook(() => - useMessageQueue({ - isConfigInitialized: true, - streamingState: StreamingState.Responding, - submitQuery: mockSubmitQuery, - }), - ); + const { result } = renderHook(() => useMessageQueue()); act(() => { result.current.addMessage('a'); @@ -275,13 +222,7 @@ describe('useMessageQueue', () => { describe('popNextSegment', () => { it('returns null when the queue is empty', () => { - const { result } = renderHook(() => - useMessageQueue({ - isConfigInitialized: true, - streamingState: StreamingState.Idle, - submitQuery: mockSubmitQuery, - }), - ); + const { result } = renderHook(() => useMessageQueue()); let segment: string | null = null; act(() => { @@ -290,64 +231,12 @@ describe('useMessageQueue', () => { expect(segment).toBeNull(); }); - it('batches leading plain-text messages into one segment', () => { - const { result } = renderHook(() => - useMessageQueue({ - isConfigInitialized: true, - streamingState: StreamingState.Responding, - submitQuery: mockSubmitQuery, - }), - ); + it('pops the first item and leaves the rest queued', () => { + const { result } = renderHook(() => useMessageQueue()); act(() => { - result.current.addMessage('hello'); - result.current.addMessage('world'); - }); - - let segment: string | null = null; - act(() => { - segment = result.current.popNextSegment(); - }); - expect(segment).toBe('hello\n\nworld'); - expect(result.current.messageQueue).toEqual([]); - }); - - it('stops batching at the first slash command and leaves it queued', () => { - const { result } = renderHook(() => - useMessageQueue({ - isConfigInitialized: true, - streamingState: StreamingState.Responding, - submitQuery: mockSubmitQuery, - }), - ); - - act(() => { - result.current.addMessage('hello'); - result.current.addMessage('world'); result.current.addMessage('/model'); - result.current.addMessage('after'); - }); - - let segment: string | null = null; - act(() => { - segment = result.current.popNextSegment(); - }); - expect(segment).toBe('hello\n\nworld'); - expect(result.current.messageQueue).toEqual(['/model', 'after']); - }); - - it('returns a slash command alone when it leads the queue', () => { - const { result } = renderHook(() => - useMessageQueue({ - isConfigInitialized: true, - streamingState: StreamingState.Responding, - submitQuery: mockSubmitQuery, - }), - ); - - act(() => { - result.current.addMessage('/model'); - result.current.addMessage('hello'); + result.current.addMessage('/help'); }); let segment: string | null = null; @@ -355,22 +244,16 @@ describe('useMessageQueue', () => { segment = result.current.popNextSegment(); }); expect(segment).toBe('/model'); - expect(result.current.messageQueue).toEqual(['hello']); + expect(result.current.messageQueue).toEqual(['/help']); }); - it('drains segments one at a time across repeated calls', () => { - const { result } = renderHook(() => - useMessageQueue({ - isConfigInitialized: true, - streamingState: StreamingState.Responding, - submitQuery: mockSubmitQuery, - }), - ); + it('drains the queue one item at a time across repeated calls', () => { + const { result } = renderHook(() => useMessageQueue()); act(() => { - result.current.addMessage('hello'); result.current.addMessage('/model'); - result.current.addMessage('world'); + result.current.addMessage('/theme'); + result.current.addMessage('/help'); }); const segments: Array = []; @@ -387,36 +270,8 @@ describe('useMessageQueue', () => { segments.push(result.current.popNextSegment()); }); - expect(segments).toEqual(['hello', '/model', 'world', null]); + expect(segments).toEqual(['/model', '/theme', '/help', null]); expect(result.current.messageQueue).toEqual([]); }); - - it('preserves remaining messages so popAllMessages can restore them after a cancel', () => { - const { result } = renderHook(() => - useMessageQueue({ - isConfigInitialized: true, - streamingState: StreamingState.Responding, - submitQuery: mockSubmitQuery, - }), - ); - - act(() => { - result.current.addMessage('hello'); - result.current.addMessage('/model'); - result.current.addMessage('after'); - }); - - act(() => { - result.current.popNextSegment(); - }); - expect(result.current.messageQueue).toEqual(['/model', 'after']); - - let popped: string | null = null; - act(() => { - popped = result.current.popAllMessages(); - }); - expect(popped).toBe('/model'); - expect(result.current.messageQueue).toEqual(['after']); - }); }); }); diff --git a/packages/cli/src/ui/hooks/useMessageQueue.ts b/packages/cli/src/ui/hooks/useMessageQueue.ts index 944fb39af50..348bfa07008 100644 --- a/packages/cli/src/ui/hooks/useMessageQueue.ts +++ b/packages/cli/src/ui/hooks/useMessageQueue.ts @@ -5,72 +5,26 @@ */ import { useCallback, useRef, useState } from 'react'; -import type { StreamingState } from '../types.js'; import { isSlashCommand } from '../utils/commandUtils.js'; -export interface UseMessageQueueOptions { - isConfigInitialized: boolean; - streamingState: StreamingState; - submitQuery: (query: string) => void | Promise; -} - -// Extract the first queue segment in original order. A segment is either a -// single slash command or a batch of consecutive plain-text prompts joined -// with `\n\n`. -function extractFirstSegment(messages: string[]): { - segment: string; - rest: string[]; -} { - if (isSlashCommand(messages[0])) { - return { segment: messages[0], rest: messages.slice(1) }; - } - const slashIdx = messages.findIndex((m) => isSlashCommand(m)); - if (slashIdx === -1) { - return { segment: messages.join('\n\n'), rest: [] }; - } - return { - segment: messages.slice(0, slashIdx).join('\n\n'), - rest: messages.slice(slashIdx), - }; -} - export interface UseMessageQueueReturn { messageQueue: string[]; addMessage: (message: string) => void; clearQueue: () => void; getQueuedMessagesText: () => string; + /** Drain the entire queue joined with `\n\n`. For Ctrl+C / ESC / Up edit-restore. */ popAllMessages: () => string | null; - /** - * Atomically drain every queued plain-text prompt from the queue while - * leaving slash commands deferred for manual execution. Returns the - * drained prompts in their original order and updates both the - * synchronous ref and React state. - * Safe to call from non-React contexts (e.g., tool completion callbacks). - */ + /** Drain plain-text prompts; leave slash commands queued. Safe from non-React callbacks. */ drainQueue: () => string[]; - /** - * Pop the next submission segment from the queue. Consecutive plain-text - * messages are batched into one segment joined by `\n\n`; slash commands - * are returned alone so the receiver's `isSlashCommand` check still fires. - * Returns `null` when the queue is empty. - */ + /** Pop the first item from the queue. */ popNextSegment: () => string | null; } -/** - * Hook for managing message queuing during streaming responses. - * Allows users to queue messages while the AI is responding and automatically - * sends them when streaming completes. - */ -export function useMessageQueue( - _options: UseMessageQueueOptions, -): UseMessageQueueReturn { +export function useMessageQueue(): UseMessageQueueReturn { const [messageQueue, setMessageQueue] = useState([]); - // Synchronous ref mirrors React state so non-React callbacks (e.g., - // mid-turn drain in handleCompletedTools) always see the latest queue. + // Synchronous mirror so non-React callbacks see the latest queue. const queueRef = useRef([]); - // Add a message to the queue const addMessage = useCallback((message: string) => { const trimmedMessage = message.trim(); if (trimmedMessage.length > 0) { @@ -79,31 +33,24 @@ export function useMessageQueue( } }, []); - // Clear the entire queue const clearQueue = useCallback(() => { queueRef.current = []; setMessageQueue([]); }, []); - // Get all queued messages as a single text string const getQueuedMessagesText = useCallback(() => { if (messageQueue.length === 0) return ''; return messageQueue.join('\n\n'); }, [messageQueue]); - // Pop deferred queue content for editing (atomic via ref to prevent - // duplicate pops from key auto-repeat before React re-renders). const popAllMessages = useCallback((): string | null => { const current = queueRef.current; if (current.length === 0) return null; - const { segment, rest } = extractFirstSegment(current); - queueRef.current = rest; - setMessageQueue(rest); - return segment; + queueRef.current = []; + setMessageQueue([]); + return current.join('\n\n'); }, []); - // Atomically drain every plain-text prompt (synchronous, safe from - // callbacks) while leaving slash commands deferred for manual execution. const drainQueue = useCallback((): string[] => { const current = queueRef.current; if (current.length === 0) return []; @@ -115,15 +62,13 @@ export function useMessageQueue( return drained; }, []); - // Pop the next submission segment. Caller is responsible for gating on - // streamingState, open dialogs, etc.; this hook only owns queue state. const popNextSegment = useCallback((): string | null => { const current = queueRef.current; if (current.length === 0) return null; - const { segment, rest } = extractFirstSegment(current); + const [head, ...rest] = current; queueRef.current = rest; setMessageQueue(rest); - return segment; + return head; }, []); return {