From 0a1ffd98ebdc167ba0724d1a30e1732f24feef41 Mon Sep 17 00:00:00 2001 From: yiliang114 <1204183885@qq.com> Date: Fri, 20 Mar 2026 00:25:51 +0800 Subject: [PATCH] feat(cli): make /btw command non-blocking with parallel execution - Add btwItem state management independent from pendingItem - Add cancelBtw functionality to abort in-flight BTW API calls - Allow /btw commands to execute concurrently with main responses - Add isBtwCommand utility function - Update BtwMessage UI with cleaner styling (remove spinner) - Add tests for concurrent /btw execution scenarios - Update layouts to render BTW messages in fixed bottom area Co-authored-by: Qwen-Coder --- .../cli/src/nonInteractiveCliCommands.test.ts | 27 ++ packages/cli/src/nonInteractiveCliCommands.ts | 1 + .../cli/src/test-utils/mockCommandContext.ts | 4 + packages/cli/src/ui/AppContainer.test.tsx | 35 +++ packages/cli/src/ui/AppContainer.tsx | 38 ++- .../cli/src/ui/commands/btwCommand.test.ts | 232 ++++++++++-------- packages/cli/src/ui/commands/btwCommand.ts | 56 +++-- packages/cli/src/ui/commands/types.ts | 11 +- .../src/ui/components/messages/BtwMessage.tsx | 46 ++-- .../cli/src/ui/contexts/UIStateContext.tsx | 4 + .../cli/src/ui/hooks/slashCommandProcessor.ts | 23 +- .../cli/src/ui/hooks/useGeminiStream.test.tsx | 107 +++++++- packages/cli/src/ui/hooks/useGeminiStream.ts | 35 ++- .../cli/src/ui/layouts/DefaultAppLayout.tsx | 7 + .../src/ui/layouts/ScreenReaderAppLayout.tsx | 8 + .../src/ui/noninteractive/nonInteractiveUi.ts | 4 + packages/cli/src/ui/utils/commandUtils.ts | 15 ++ 17 files changed, 497 insertions(+), 156 deletions(-) diff --git a/packages/cli/src/nonInteractiveCliCommands.test.ts b/packages/cli/src/nonInteractiveCliCommands.test.ts index 76b29f3e0..c1c47c678 100644 --- a/packages/cli/src/nonInteractiveCliCommands.test.ts +++ b/packages/cli/src/nonInteractiveCliCommands.test.ts @@ -149,6 +149,33 @@ describe('handleSlashCommand', () => { } }); + it('should execute /btw when using the default allowed list', async () => { + const mockBtwCommand = { + name: 'btw', + description: 'Ask a side question', + kind: CommandKind.BUILT_IN, + action: vi.fn().mockResolvedValue({ + type: 'message', + messageType: 'info', + content: 'btw> question\nanswer', + }), + }; + mockGetCommands.mockReturnValue([mockBtwCommand]); + + const result = await handleSlashCommand( + '/btw question', + abortController, + mockConfig, + mockSettings, + ); + + expect(mockBtwCommand.action).toHaveBeenCalled(); + expect(result.type).toBe('message'); + if (result.type === 'message') { + expect(result.content).toBe('btw> question\nanswer'); + } + }); + it('should execute file commands regardless of allowed list', async () => { const mockFileCommand = { name: 'custom', diff --git a/packages/cli/src/nonInteractiveCliCommands.ts b/packages/cli/src/nonInteractiveCliCommands.ts index b089fa6c2..e6344f5d0 100644 --- a/packages/cli/src/nonInteractiveCliCommands.ts +++ b/packages/cli/src/nonInteractiveCliCommands.ts @@ -42,6 +42,7 @@ export const ALLOWED_BUILTIN_COMMANDS_NON_INTERACTIVE = [ 'init', 'summary', 'compress', + 'btw', 'bug', ] as const; diff --git a/packages/cli/src/test-utils/mockCommandContext.ts b/packages/cli/src/test-utils/mockCommandContext.ts index fd825b9df..d6a6c3e6d 100644 --- a/packages/cli/src/test-utils/mockCommandContext.ts +++ b/packages/cli/src/test-utils/mockCommandContext.ts @@ -55,6 +55,10 @@ export const createMockCommandContext = ( setDebugMessage: vi.fn(), pendingItem: null, setPendingItem: vi.fn(), + btwItem: null, + setBtwItem: vi.fn(), + cancelBtw: vi.fn(), + btwAbortControllerRef: { current: null }, loadHistory: vi.fn(), toggleVimEnabled: vi.fn(), extensionsUpdateState: new Map(), diff --git a/packages/cli/src/ui/AppContainer.test.tsx b/packages/cli/src/ui/AppContainer.test.tsx index 9e9d4f673..5601fc836 100644 --- a/packages/cli/src/ui/AppContainer.test.tsx +++ b/packages/cli/src/ui/AppContainer.test.tsx @@ -419,6 +419,41 @@ describe('AppContainer State Management', () => { ); }).not.toThrow(); }); + + it('submits /btw immediately instead of queueing while responding', () => { + const mockSubmitQuery = vi.fn(); + const mockQueueMessage = vi.fn(); + + mockedUseGeminiStream.mockReturnValue({ + streamingState: 'responding', + 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(''), + }); + + render( + , + ); + + capturedUIActions.handleFinalSubmit('/btw quick side question'); + + expect(mockSubmitQuery).toHaveBeenCalledWith('/btw quick side question'); + expect(mockQueueMessage).not.toHaveBeenCalled(); + }); }); describe('Settings Integration', () => { diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index c6bfa67c3..e5f83ed4b 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -68,6 +68,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 { type LoadedSettings, SettingScope } from '../config/settings.js'; import { type InitializationResult } from '../core/initializer.js'; import { useFocus } from './hooks/useFocus.js'; @@ -550,6 +551,9 @@ export const AppContainer = (props: AppContainerProps) => { handleSlashCommand, slashCommands, pendingHistoryItems: pendingSlashCommandHistoryItems, + btwItem, + setBtwItem, + cancelBtw, commandContext, shellConfirmationRequest, confirmationRequest, @@ -687,9 +691,16 @@ export const AppContainer = (props: AppContainerProps) => { // Callback for handling final submit (must be after addMessage from useMessageQueue) const handleFinalSubmit = useCallback( (submittedValue: string) => { + if ( + streamingState === StreamingState.Responding && + isBtwCommand(submittedValue) + ) { + void submitQuery(submittedValue); + return; + } addMessage(submittedValue); }, - [addMessage], + [addMessage, streamingState, submitQuery], ); // Welcome back functionality (must be after handleFinalSubmit) @@ -1148,7 +1159,12 @@ export const AppContainer = (props: AppContainerProps) => { handleExit(ctrlDPressedOnce, setCtrlDPressedOnce, ctrlDTimerRef); return; } else if (keyMatchers[Command.ESCAPE](key)) { - // Escape key handling + // Dismiss or cancel btw side-question on Escape + if (btwItem) { + cancelBtw(); + return; + } + // Skip if shell is focused (to allow shell's own escape handling) if (embeddedShellFocused) { return; @@ -1190,6 +1206,15 @@ export const AppContainer = (props: AppContainerProps) => { return; } + // Dismiss completed btw side-question on Space or Enter, + // but only when the input buffer is empty so we don't swallow user keystrokes. + if (btwItem && !btwItem.btw.isPending && buffer.text.length === 0) { + if (key.name === 'return' || key.sequence === ' ') { + setBtwItem(null); + return; + } + } + let enteringConstrainHeightMode = false; if (!constrainHeight) { enteringConstrainHeightMode = true; @@ -1244,6 +1269,9 @@ export const AppContainer = (props: AppContainerProps) => { handleSlashCommand, activePtyId, embeddedShellFocused, + btwItem, + setBtwItem, + cancelBtw, settings.merged.general?.debugKeystrokeLogging, isAuthenticating, ], @@ -1403,6 +1431,9 @@ export const AppContainer = (props: AppContainerProps) => { staticExtraHeight, dialogsVisible, pendingHistoryItems, + btwItem, + setBtwItem, + cancelBtw, nightly, branchName, sessionStats, @@ -1495,6 +1526,9 @@ export const AppContainer = (props: AppContainerProps) => { staticExtraHeight, dialogsVisible, pendingHistoryItems, + btwItem, + setBtwItem, + cancelBtw, nightly, branchName, sessionStats, diff --git a/packages/cli/src/ui/commands/btwCommand.test.ts b/packages/cli/src/ui/commands/btwCommand.test.ts index a0ee20ec4..99dfa40d3 100644 --- a/packages/cli/src/ui/commands/btwCommand.test.ts +++ b/packages/cli/src/ui/commands/btwCommand.test.ts @@ -27,6 +27,15 @@ describe('btwCommand', () => { let mockContext: CommandContext; let mockGenerateContent: ReturnType; let mockGetHistory: ReturnType; + const createConfig = (overrides: Record = {}) => ({ + getGeminiClient: () => ({ + getHistory: mockGetHistory, + generateContent: mockGenerateContent, + }), + getModel: () => 'test-model', + getSessionId: () => 'test-session-id', + ...overrides, + }); beforeEach(() => { vi.clearAllMocks(); @@ -36,13 +45,7 @@ describe('btwCommand', () => { mockContext = createMockCommandContext({ services: { - config: { - getGeminiClient: () => ({ - getHistory: mockGetHistory, - generateContent: mockGenerateContent, - }), - getModel: () => 'test-model', - }, + config: createConfig(), }, }); }); @@ -90,13 +93,9 @@ describe('btwCommand', () => { it('should return error when model is not configured', async () => { const noModelContext = createMockCommandContext({ services: { - config: { - getGeminiClient: () => ({ - getHistory: mockGetHistory, - generateContent: mockGenerateContent, - }), + config: createConfig({ getModel: () => '', - }, + }), }, }); @@ -110,11 +109,10 @@ describe('btwCommand', () => { }); describe('interactive mode', () => { - // Helper to flush microtask queue so fire-and-forget promises settle. const flushPromises = () => new Promise((resolve) => setTimeout(resolve, 0)); - it('should set pending item and add completed item on success', async () => { + it('should set btwItem and update it on success', async () => { mockGenerateContent.mockResolvedValue({ candidates: [ { @@ -127,8 +125,8 @@ describe('btwCommand', () => { await btwCommand.action!(mockContext, 'what is the meaning of life?'); - // Action returns immediately; pending item is set synchronously - expect(mockContext.ui.setPendingItem).toHaveBeenCalledWith({ + // Action returns immediately; btwItem is set synchronously + expect(mockContext.ui.setBtwItem).toHaveBeenCalledWith({ type: MessageType.BTW, btw: { question: 'what is the meaning of life?', @@ -137,22 +135,23 @@ describe('btwCommand', () => { }, }); - // Wait for background promise to settle + // pendingItem should NOT be used + expect(mockContext.ui.setPendingItem).not.toHaveBeenCalled(); + await flushPromises(); - expect(mockContext.ui.addItem).toHaveBeenCalledWith( - { - type: MessageType.BTW, - btw: { - question: 'what is the meaning of life?', - answer: 'The answer is 42.', - isPending: false, - }, + // On success, setBtwItem is called with the completed answer + expect(mockContext.ui.setBtwItem).toHaveBeenCalledWith({ + type: MessageType.BTW, + btw: { + question: 'what is the meaning of life?', + answer: 'The answer is 42.', + isPending: false, }, - expect.any(Number), - ); + }); - expect(mockContext.ui.setPendingItem).toHaveBeenLastCalledWith(null); + // addItem should NOT be called (btw stays in fixed area, not in history) + expect(mockContext.ui.addItem).not.toHaveBeenCalled(); }); it('should pass conversation history to generateContent', async () => { @@ -183,15 +182,20 @@ describe('btwCommand', () => { {}, expect.any(AbortSignal), 'test-model', + expect.stringMatching(/^test-session-id########btw-/), ); }); - it('should add error item on failure', async () => { + it('should add error item on failure and clear btwItem', async () => { mockGenerateContent.mockRejectedValue(new Error('API error')); await btwCommand.action!(mockContext, 'test question'); await flushPromises(); + // btwItem should be cleared on error + expect(mockContext.ui.setBtwItem).toHaveBeenCalledWith(null); + + // Error goes to history expect(mockContext.ui.addItem).toHaveBeenCalledWith( { type: MessageType.ERROR, @@ -199,8 +203,6 @@ describe('btwCommand', () => { }, expect.any(Number), ); - - expect(mockContext.ui.setPendingItem).toHaveBeenLastCalledWith(null); }); it('should handle non-Error exceptions', async () => { @@ -218,58 +220,106 @@ describe('btwCommand', () => { ); }); - it('should return error when another operation is pending', async () => { + it('should not block when another pendingItem exists', async () => { const busyContext = createMockCommandContext({ services: { - config: { - getGeminiClient: () => ({ - getHistory: mockGetHistory, - generateContent: mockGenerateContent, - }), - getModel: () => 'test-model', - }, + config: createConfig(), }, ui: { pendingItem: { type: 'info' }, }, }); - const result = await btwCommand.action!(busyContext, 'test question'); - - expect(result).toEqual({ - type: 'message', - messageType: 'error', - content: - 'Another operation is in progress. Please wait for it to complete.', - }); - }); - - it('should not add item when abort signal is aborted', async () => { - const abortController = new AbortController(); - abortController.abort(); - - const abortContext = createMockCommandContext({ - abortSignal: abortController.signal, - services: { - config: { - getGeminiClient: () => ({ - getHistory: mockGetHistory, - generateContent: mockGenerateContent, - }), - getModel: () => 'test-model', - }, - }, - }); - mockGenerateContent.mockResolvedValue({ candidates: [{ content: { parts: [{ text: 'answer' }] } }], }); - await btwCommand.action!(abortContext, 'test question'); + // btw should NOT be blocked by pendingItem anymore + const result = await btwCommand.action!(busyContext, 'test question'); + expect(result).toBeUndefined(); + expect(busyContext.ui.setBtwItem).toHaveBeenCalled(); + }); + + it('should not update btwItem when cancelled via btwAbortControllerRef', async () => { + mockGenerateContent.mockImplementation( + () => + new Promise((resolve) => + setTimeout( + () => + resolve({ + candidates: [ + { content: { parts: [{ text: 'late answer' }] } }, + ], + }), + 50, + ), + ), + ); + + await btwCommand.action!(mockContext, 'test question'); + + // The btw command should have registered its AbortController + expect(mockContext.ui.btwAbortControllerRef.current).toBeInstanceOf( + AbortController, + ); + + // Simulate user pressing ESC: cancel the in-flight btw + mockContext.ui.btwAbortControllerRef.current!.abort(); + await flushPromises(); - expect(abortContext.ui.addItem).not.toHaveBeenCalled(); - expect(abortContext.ui.setPendingItem).toHaveBeenLastCalledWith(null); + // setBtwItem should only have the initial pending call (no completion) + expect(mockContext.ui.setBtwItem).toHaveBeenCalledTimes(1); + expect(mockContext.ui.addItem).not.toHaveBeenCalled(); + }); + + it('should clear btwAbortControllerRef after successful completion', async () => { + mockGenerateContent.mockResolvedValue({ + candidates: [{ content: { parts: [{ text: 'answer' }] } }], + }); + + await btwCommand.action!(mockContext, 'test question'); + + // Ref is set during the call + expect(mockContext.ui.btwAbortControllerRef.current).toBeInstanceOf( + AbortController, + ); + + await flushPromises(); + + // After completion, ref should be cleaned up + expect(mockContext.ui.btwAbortControllerRef.current).toBeNull(); + }); + + it('should clear btwAbortControllerRef after error', async () => { + mockGenerateContent.mockRejectedValue(new Error('API error')); + + await btwCommand.action!(mockContext, 'test question'); + + expect(mockContext.ui.btwAbortControllerRef.current).toBeInstanceOf( + AbortController, + ); + + await flushPromises(); + + expect(mockContext.ui.btwAbortControllerRef.current).toBeNull(); + }); + + it('should cancel previous btw when starting a new one', async () => { + mockGenerateContent.mockResolvedValue({ + candidates: [{ content: { parts: [{ text: 'answer' }] } }], + }); + + await btwCommand.action!(mockContext, 'first question'); + + // cancelBtw should have been called to clean up any previous btw + expect(mockContext.ui.cancelBtw).toHaveBeenCalledTimes(1); + + // Second btw call + await btwCommand.action!(mockContext, 'second question'); + + // cancelBtw called again for the second invocation + expect(mockContext.ui.cancelBtw).toHaveBeenCalledTimes(2); }); it('should return fallback text when response has no parts', async () => { @@ -280,17 +330,14 @@ describe('btwCommand', () => { await btwCommand.action!(mockContext, 'test question'); await flushPromises(); - expect(mockContext.ui.addItem).toHaveBeenCalledWith( - { - type: MessageType.BTW, - btw: { - question: 'test question', - answer: 'No response received.', - isPending: false, - }, + expect(mockContext.ui.setBtwItem).toHaveBeenCalledWith({ + type: MessageType.BTW, + btw: { + question: 'test question', + answer: 'No response received.', + isPending: false, }, - expect.any(Number), - ); + }); }); it('should return void immediately without blocking', async () => { @@ -300,16 +347,15 @@ describe('btwCommand', () => { const result = await btwCommand.action!(mockContext, 'test question'); - // Action should return void (not awaiting the API call) expect(result).toBeUndefined(); - // addItem not yet called — background promise hasn't settled - expect(mockContext.ui.addItem).not.toHaveBeenCalled(); + // Only the pending setBtwItem called so far + expect(mockContext.ui.setBtwItem).toHaveBeenCalledTimes(1); await flushPromises(); - // Now the background work has completed - expect(mockContext.ui.addItem).toHaveBeenCalled(); + // Now the completed setBtwItem has been called + expect(mockContext.ui.setBtwItem).toHaveBeenCalledTimes(2); }); }); @@ -320,13 +366,7 @@ describe('btwCommand', () => { nonInteractiveContext = createMockCommandContext({ executionMode: 'non_interactive', services: { - config: { - getGeminiClient: () => ({ - getHistory: mockGetHistory, - generateContent: mockGenerateContent, - }), - getModel: () => 'test-model', - }, + config: createConfig(), }, }); }); @@ -371,13 +411,7 @@ describe('btwCommand', () => { acpContext = createMockCommandContext({ executionMode: 'acp', services: { - config: { - getGeminiClient: () => ({ - getHistory: mockGetHistory, - generateContent: mockGenerateContent, - }), - getModel: () => 'test-model', - }, + config: createConfig(), }, }); }); diff --git a/packages/cli/src/ui/commands/btwCommand.ts b/packages/cli/src/ui/commands/btwCommand.ts index 9350914ce..7ee5668df 100644 --- a/packages/cli/src/ui/commands/btwCommand.ts +++ b/packages/cli/src/ui/commands/btwCommand.ts @@ -15,6 +15,10 @@ import type { HistoryItemBtw } from '../types.js'; import { t } from '../../i18n/index.js'; import type { GeminiClient } from '@qwen-code/qwen-code-core'; +function makeBtwPromptId(sessionId: string): string { + return `${sessionId}########btw-${Date.now()}`; +} + function formatBtwError(error: unknown): string { return t('Failed to answer btw question: {{error}}', { error: error instanceof Error ? error.message : String(error), @@ -30,6 +34,7 @@ async function askBtw( model: string, question: string, abortSignal: AbortSignal, + promptId: string, ): Promise { const history = geminiClient.getHistory(); @@ -45,9 +50,10 @@ async function askBtw( ], }, ], - {}, // No tools — btw questions are text-only + {}, abortSignal, model, + promptId, ); const parts = response.candidates?.[0]?.content?.parts; @@ -96,6 +102,7 @@ export const btwCommand: SlashCommand = { const geminiClient = config.getGeminiClient(); const model = config.getModel(); + const sessionId = config.getSessionId(); if (!model) { return { @@ -107,6 +114,7 @@ export const btwCommand: SlashCommand = { // ACP mode: return a stream_messages async generator if (executionMode === 'acp') { + const btwPromptId = makeBtwPromptId(sessionId); const messages = async function* () { try { yield { @@ -119,6 +127,7 @@ export const btwCommand: SlashCommand = { model, question, abortSignal, + btwPromptId, ); yield { @@ -139,7 +148,14 @@ export const btwCommand: SlashCommand = { // Non-interactive mode: return a simple message result if (executionMode === 'non_interactive') { try { - const answer = await askBtw(geminiClient, model, question, abortSignal); + const btwPromptId = makeBtwPromptId(sessionId); + const answer = await askBtw( + geminiClient, + model, + question, + abortSignal, + btwPromptId, + ); return { type: 'message', messageType: 'info', @@ -154,16 +170,15 @@ export const btwCommand: SlashCommand = { } } - // Interactive mode: use pending item for spinner, then add to UI history - if (ui.pendingItem) { - return { - type: 'message', - messageType: 'error', - content: t( - 'Another operation is in progress. Please wait for it to complete.', - ), - }; - } + // Interactive mode: use dedicated btwItem state for the fixed bottom area. + // This does NOT occupy pendingItem, so the main conversation is never blocked. + + // Cancel any previous in-flight btw before starting a new one. + ui.cancelBtw(); + + const btwAbortController = new AbortController(); + const btwSignal = btwAbortController.signal; + ui.btwAbortControllerRef.current = btwAbortController; const pendingItem: HistoryItemBtw = { type: MessageType.BTW, @@ -173,14 +188,16 @@ export const btwCommand: SlashCommand = { isPending: true, }, }; - ui.setPendingItem(pendingItem); + ui.setBtwItem(pendingItem); // Fire-and-forget: run the API call in the background so the main // conversation is not blocked while waiting for the btw answer. - void askBtw(geminiClient, model, question, abortSignal) + const btwPromptId = makeBtwPromptId(sessionId); + void askBtw(geminiClient, model, question, btwSignal, btwPromptId) .then((answer) => { - if (abortSignal.aborted) return; + if (btwSignal.aborted) return; + ui.btwAbortControllerRef.current = null; const completedItem: HistoryItemBtw = { type: MessageType.BTW, btw: { @@ -189,11 +206,13 @@ export const btwCommand: SlashCommand = { isPending: false, }, }; - ui.addItem(completedItem, Date.now()); + ui.setBtwItem(completedItem); }) .catch((error) => { - if (abortSignal.aborted) return; + if (btwSignal.aborted) return; + ui.btwAbortControllerRef.current = null; + ui.setBtwItem(null); ui.addItem( { type: MessageType.ERROR, @@ -201,9 +220,6 @@ export const btwCommand: SlashCommand = { }, Date.now(), ); - }) - .finally(() => { - ui.setPendingItem(null); }); }, }; diff --git a/packages/cli/src/ui/commands/types.ts b/packages/cli/src/ui/commands/types.ts index 76eda2c07..3fe41647b 100644 --- a/packages/cli/src/ui/commands/types.ts +++ b/packages/cli/src/ui/commands/types.ts @@ -4,12 +4,13 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type { ReactNode } from 'react'; +import type { MutableRefObject, ReactNode } from 'react'; import type { Content, PartListUnion } from '@google/genai'; import type { Config, GitService, Logger } from '@qwen-code/qwen-code-core'; import type { HistoryItemWithoutId, HistoryItem, + HistoryItemBtw, ConfirmationRequest, } from '../types.js'; import type { LoadedSettings } from '../../config/settings.js'; @@ -66,6 +67,14 @@ export interface CommandContext { * @param item The history item to display as pending, or `null` to clear. */ setPendingItem: (item: HistoryItemWithoutId | null) => void; + /** The current btw side-question item rendered in the fixed bottom area. */ + btwItem: HistoryItemBtw | null; + /** Sets the btw item independently of the main pendingItem. */ + setBtwItem: (item: HistoryItemBtw | null) => void; + /** Cancels a pending btw (aborts the in-flight API call and clears the btw area). */ + cancelBtw: () => void; + /** Ref to the btw AbortController, set by btwCommand so cancelBtw can abort it. */ + btwAbortControllerRef: MutableRefObject; /** * Loads a new set of history items, replacing the current history. * diff --git a/packages/cli/src/ui/components/messages/BtwMessage.tsx b/packages/cli/src/ui/components/messages/BtwMessage.tsx index 97d0085e0..8a7ac9d15 100644 --- a/packages/cli/src/ui/components/messages/BtwMessage.tsx +++ b/packages/cli/src/ui/components/messages/BtwMessage.tsx @@ -7,7 +7,6 @@ import type React from 'react'; import { Box, Text } from 'ink'; import type { BtwProps } from '../../types.js'; -import Spinner from 'ink-spinner'; import { Colors } from '../../colors.js'; import { t } from '../../../i18n/index.js'; @@ -15,35 +14,34 @@ export interface BtwDisplayProps { btw: BtwProps; } -/** - * BtwMessage renders the /btw (by the way) sidebar response. - * Shows an ephemeral question and answer that doesn't affect the main conversation. - */ export const BtwMessage: React.FC = ({ btw }) => ( - + - - {'btw> '} + + {'/btw '} - + {btw.question} - - {btw.isPending ? ( - - - - - {t('Thinking...')} + {btw.isPending ? ( + + {'+ '} + {t('Answering...')} + + ) : ( + + {btw.answer} + + {t('Press Space, Enter, or Escape to dismiss')} - ) : ( - - - {btw.answer} - - - )} - + + )} ); diff --git a/packages/cli/src/ui/contexts/UIStateContext.tsx b/packages/cli/src/ui/contexts/UIStateContext.tsx index 0d461e70c..7f2e25ec7 100644 --- a/packages/cli/src/ui/contexts/UIStateContext.tsx +++ b/packages/cli/src/ui/contexts/UIStateContext.tsx @@ -7,6 +7,7 @@ import { createContext, useContext } from 'react'; import type { HistoryItem, + HistoryItemBtw, ThoughtSummary, ShellConfirmationRequest, ConfirmationRequest, @@ -101,6 +102,9 @@ export interface UIState { staticExtraHeight: number; dialogsVisible: boolean; pendingHistoryItems: HistoryItemWithoutId[]; + btwItem: HistoryItemBtw | null; + setBtwItem: (item: HistoryItemBtw | null) => void; + cancelBtw: () => void; nightly: boolean; branchName: string | undefined; sessionStats: SessionStatsState; diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.ts index b22e35909..bcdeaa34c 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.ts @@ -22,6 +22,7 @@ import { useSessionStats } from '../contexts/SessionContext.js'; import type { Message, HistoryItemWithoutId, + HistoryItemBtw, SlashCommandProcessorResult, HistoryItem, ConfirmationRequest, @@ -137,10 +138,20 @@ export const useSlashCommandProcessor = ( null, ); + const [btwItem, setBtwItem] = useState(null); + const btwAbortControllerRef = useRef(null); + + const cancelBtw = useCallback(() => { + btwAbortControllerRef.current?.abort(); + btwAbortControllerRef.current = null; + setBtwItem(null); + }, []); + // AbortController for cancelling async slash commands via ESC const abortControllerRef = useRef(null); const cancelSlashCommand = useCallback(() => { + cancelBtw(); if (!abortControllerRef.current) { return; } @@ -154,7 +165,7 @@ export const useSlashCommandProcessor = ( ); setPendingItem(null); setIsProcessing(false); - }, [addItem, setIsProcessing]); + }, [addItem, setIsProcessing, cancelBtw]); useKeypress( (key) => { @@ -249,6 +260,10 @@ export const useSlashCommandProcessor = ( setDebugMessage: actions.setDebugMessage, pendingItem, setPendingItem, + btwItem, + setBtwItem, + cancelBtw, + btwAbortControllerRef, toggleVimEnabled, setGeminiMdFileCount, reloadCommands, @@ -277,6 +292,9 @@ export const useSlashCommandProcessor = ( actions, pendingItem, setPendingItem, + btwItem, + setBtwItem, + cancelBtw, toggleVimEnabled, sessionShellAllowlist, setGeminiMdFileCount, @@ -710,6 +728,9 @@ export const useSlashCommandProcessor = ( handleSlashCommand, slashCommands: commands, pendingHistoryItems, + btwItem, + setBtwItem, + cancelBtw, commandContext, shellConfirmationRequest, confirmationRequest, diff --git a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx index e6696ae6b..49af6521e 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx +++ b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx @@ -832,7 +832,7 @@ describe('useGeminiStream', () => { // Wait for the first part of the response await waitFor(() => { - expect(result.current.streamingState).toBe(StreamingState.Responding); + expect(mockSendMessageStream).toHaveBeenCalledTimes(1); }); // Call cancelOngoingRequest directly @@ -981,7 +981,7 @@ describe('useGeminiStream', () => { }); await waitFor(() => { - expect(result.current.streamingState).toBe(StreamingState.Responding); + expect(mockSendMessageStream).toHaveBeenCalledTimes(1); }); // Cancel the request @@ -2707,6 +2707,109 @@ describe('useGeminiStream', () => { }); describe('Concurrent Execution Prevention', () => { + it('should allow /btw slash commands while a main response is in progress', async () => { + let resolveFirstCall!: () => void; + + const firstCallPromise = new Promise((resolve) => { + resolveFirstCall = resolve; + }); + + const firstStream = (async function* () { + yield { + type: ServerGeminiEventType.Content, + value: 'First call content', + }; + await firstCallPromise; + })(); + + mockSendMessageStream.mockImplementation(() => firstStream); + mockHandleSlashCommand.mockImplementation(async (command) => { + if (command === '/btw quick side question') { + return { type: 'handled' }; + } + return false; + }); + + const { result } = renderTestHook(); + + let mainRequest!: Promise; + await act(async () => { + mainRequest = result.current.submitQuery('First query'); + }); + + try { + await waitFor(() => { + expect(mockSendMessageStream).toHaveBeenCalledTimes(1); + expect(result.current.streamingState).toBe(StreamingState.Responding); + }); + + await act(async () => { + await result.current.submitQuery('/btw quick side question'); + }); + + expect(mockHandleSlashCommand).toHaveBeenCalledWith( + '/btw quick side question', + ); + expect(mockSendMessageStream).toHaveBeenCalledTimes(1); + } finally { + resolveFirstCall(); + await mainRequest; + } + }); + + it('should keep the main request cancellable after submitting /btw in parallel', async () => { + let resolveFirstCall!: () => void; + let mainAbortSignal: AbortSignal | undefined; + + const firstCallPromise = new Promise((resolve) => { + resolveFirstCall = resolve; + }); + + mockSendMessageStream.mockImplementation((_query, signal) => { + mainAbortSignal = signal; + return (async function* () { + yield { + type: ServerGeminiEventType.Content, + value: 'First call content', + }; + await firstCallPromise; + })(); + }); + mockHandleSlashCommand.mockImplementation(async (command) => { + if (command === '/btw quick side question') { + return { type: 'handled' }; + } + return false; + }); + + const { result } = renderTestHook(); + + let mainRequest!: Promise; + await act(async () => { + mainRequest = result.current.submitQuery('First query'); + }); + + try { + await waitFor(() => { + expect(mainAbortSignal).toBeDefined(); + expect(result.current.streamingState).toBe(StreamingState.Responding); + }); + + await act(async () => { + await result.current.submitQuery('/btw quick side question'); + }); + + act(() => { + result.current.cancelOngoingRequest(); + }); + + expect(mainAbortSignal?.aborted).toBe(true); + } finally { + resolveFirstCall(); + await mainRequest; + } + }); + it('should prevent concurrent submitQuery calls', async () => { let resolveFirstCall!: () => void; let resolveSecondCall!: () => void; diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index 7614eed00..1d4d736aa 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -46,7 +46,11 @@ import type { SlashCommandProcessorResult, } from '../types.js'; import { StreamingState, MessageType, ToolCallStatus } from '../types.js'; -import { isAtCommand, isSlashCommand } from '../utils/commandUtils.js'; +import { + isAtCommand, + isBtwCommand, + isSlashCommand, +} from '../utils/commandUtils.js'; import { useShellCommandProcessor } from './shellCommandProcessor.js'; import { handleAtCommand } from './atCommandProcessor.js'; import { findLastSafeSplitPoint } from '../utils/markdownUtilities.js'; @@ -1085,16 +1089,27 @@ export const useGeminiStream = ( options?: { isContinuation: boolean; skipPreparation?: boolean }, prompt_id?: string, ) => { + const allowConcurrentBtwDuringResponse = + !options?.isContinuation && + streamingState === StreamingState.Responding && + typeof query === 'string' && + isBtwCommand(query); + // Prevent concurrent executions of submitQuery, but allow continuations // which are part of the same logical flow (tool responses) - if (isSubmittingQueryRef.current && !options?.isContinuation) { + if ( + isSubmittingQueryRef.current && + !options?.isContinuation && + !allowConcurrentBtwDuringResponse + ) { return; } if ( (streamingState === StreamingState.Responding || streamingState === StreamingState.WaitingForConfirmation) && - !options?.isContinuation + !options?.isContinuation && + !allowConcurrentBtwDuringResponse ) return; @@ -1104,7 +1119,7 @@ export const useGeminiStream = ( const userMessageTimestamp = Date.now(); // Reset quota error flag when starting a new query (not a continuation) - if (!options?.isContinuation) { + if (!options?.isContinuation && !allowConcurrentBtwDuringResponse) { setModelSwitchedFromQuotaError(false); // Commit any pending retry error to history (without hint) since the // user is starting a new conversation turn. @@ -1118,9 +1133,15 @@ export const useGeminiStream = ( } } - abortControllerRef.current = new AbortController(); - const abortSignal = abortControllerRef.current.signal; - turnCancelledRef.current = false; + const abortController = new AbortController(); + const abortSignal = abortController.signal; + + // Keep the main stream's cancellation state intact while /btw is handled + // in parallel. The side-question can use its own local abort signal. + if (!allowConcurrentBtwDuringResponse) { + abortControllerRef.current = abortController; + turnCancelledRef.current = false; + } if (!prompt_id) { prompt_id = config.getSessionId() + '########' + getPromptCount(); diff --git a/packages/cli/src/ui/layouts/DefaultAppLayout.tsx b/packages/cli/src/ui/layouts/DefaultAppLayout.tsx index 93ad311c6..1dd81ecb2 100644 --- a/packages/cli/src/ui/layouts/DefaultAppLayout.tsx +++ b/packages/cli/src/ui/layouts/DefaultAppLayout.tsx @@ -10,6 +10,7 @@ import { MainContent } from '../components/MainContent.js'; import { DialogManager } from '../components/DialogManager.js'; import { Composer } from '../components/Composer.js'; import { ExitWarning } from '../components/ExitWarning.js'; +import { BtwMessage } from '../components/messages/BtwMessage.js'; import { useUIState } from '../contexts/UIStateContext.js'; import { useTerminalSize } from '../hooks/useTerminalSize.js'; @@ -21,6 +22,12 @@ export const DefaultAppLayout: React.FC = () => { + {uiState.btwItem && ( + + + + )} + {uiState.dialogsVisible ? ( diff --git a/packages/cli/src/ui/layouts/ScreenReaderAppLayout.tsx b/packages/cli/src/ui/layouts/ScreenReaderAppLayout.tsx index b4967a5f4..633f631ee 100644 --- a/packages/cli/src/ui/layouts/ScreenReaderAppLayout.tsx +++ b/packages/cli/src/ui/layouts/ScreenReaderAppLayout.tsx @@ -12,6 +12,7 @@ import { DialogManager } from '../components/DialogManager.js'; import { Composer } from '../components/Composer.js'; import { Footer } from '../components/Footer.js'; import { ExitWarning } from '../components/ExitWarning.js'; +import { BtwMessage } from '../components/messages/BtwMessage.js'; import { useUIState } from '../contexts/UIStateContext.js'; export const ScreenReaderAppLayout: React.FC = () => { @@ -24,6 +25,13 @@ export const ScreenReaderAppLayout: React.FC = () => { + + {uiState.btwItem && ( + + + + )} + {uiState.dialogsVisible ? ( {}, pendingItem: null, setPendingItem: (_item) => {}, + btwItem: null, + setBtwItem: (_item) => {}, + cancelBtw: () => {}, + btwAbortControllerRef: { current: null }, toggleVimEnabled: async () => false, setGeminiMdFileCount: (_count) => {}, reloadCommands: () => {}, diff --git a/packages/cli/src/ui/utils/commandUtils.ts b/packages/cli/src/ui/utils/commandUtils.ts index 802107f6b..69038eaad 100644 --- a/packages/cli/src/ui/utils/commandUtils.ts +++ b/packages/cli/src/ui/utils/commandUtils.ts @@ -62,6 +62,21 @@ export const isSlashCommand = (query: string): boolean => { return true; }; +/** + * Checks if a query is a /btw side-question invocation. + * Accepts both "/btw" and "?btw" prefixes. + */ +export const isBtwCommand = (query: string): boolean => { + const trimmed = query.trim(); + if (!trimmed) { + return false; + } + + const normalized = trimmed.startsWith('?') ? `/${trimmed.slice(1)}` : trimmed; + + return /^\/btw(?:\s|$)/.test(normalized); +}; + const debugLogger = createDebugLogger('COMMAND_UTILS'); // Copies a string snippet to the clipboard for different platforms