diff --git a/.changeset/calm-queues-compact.md b/.changeset/calm-queues-compact.md new file mode 100644 index 000000000..188fb47c7 --- /dev/null +++ b/.changeset/calm-queues-compact.md @@ -0,0 +1,5 @@ +--- +"@nanocollective/nanocoder": patch +--- + +Resume queued prompts after slash commands and manual context compaction complete. Refs #1060. diff --git a/source/app/App.tsx b/source/app/App.tsx index ef4081f5d..7ae3b5823 100644 --- a/source/app/App.tsx +++ b/source/app/App.tsx @@ -44,7 +44,6 @@ import {useUserMessageQueue} from '@/hooks/useUserMessageQueue'; import {useVSCodeServer} from '@/hooks/useVSCodeServer'; import {getAllSubagentProgress} from '@/services/subagent-events'; import {generateKey} from '@/session/key-generator'; -import type {ImageAttachment} from '@/types/core'; import type {ThemePreset} from '@/types/ui'; import {createPinoLogger} from '@/utils/logging/pino-logger'; import {setGlobalMessageQueue} from '@/utils/message-queue'; @@ -84,14 +83,6 @@ export default function App({ // Use extracted hooks const appState = useAppState(initialDevelopmentMode); const userMessageQueue = useUserMessageQueue(); - const queuedUserSubmitRef = React.useRef< - | (( - message: string, - displayValue: string, - images?: ImageAttachment[], - ) => Promise) - | null - >(null); const {exit} = useApp(); const {isTrusted, handleConfirmTrust, isTrustLoading, isTrustedError} = useDirectoryTrust(); @@ -249,35 +240,6 @@ export default function App({ } }, []); - const drainQueuedUserMessage = React.useCallback(() => { - // Defer to a macrotask, not a microtask. `onConversationComplete` fires - // deep inside the finishing turn's await chain, so a microtask drain would - // start the next turn BEFORE that turn's `resetStreamingState()` finally - // runs — and the stale reset would then wipe the new turn's abortController - // and isGenerating, leaving the busy indicator (and Escape-to-cancel) dead. - // A timeout runs after those continuations, so the drained turn keeps its - // busy state. - setTimeout(() => { - void userMessageQueue.drainNextMessage(async message => { - const submitQueuedMessage = queuedUserSubmitRef.current; - if (!submitQueuedMessage || !appState.client || !appState.toolManager) { - return false; - } - - await submitQueuedMessage( - message.message, - message.displayValue, - message.images, - ); - return true; - }); - }, 0); - }, [ - appState.client, - appState.toolManager, - userMessageQueue.drainNextMessage, - ]); - // Setup chat handler const chatHandler = useChatHandler({ client: appState.client, @@ -299,7 +261,6 @@ export default function App({ appState.setCompactToolCounts(null); appState.compactToolCountsRef.current = {}; appState.setLiveTaskList(null); - drainQueuedUserMessage(); }, // A turn that started in plan mode finished uninterrupted — a plan was // produced. Flag it so the interactive UI can show the plan review bar. @@ -578,10 +539,6 @@ export default function App({ activeEditor: vscodeServer.activeEditor, }); - React.useEffect(() => { - queuedUserSubmitRef.current = handleUserSubmit; - }, [handleUserSubmit]); - // Setup non-interactive mode const {nonInteractiveLoadingMessage} = useNonInteractiveMode({ nonInteractivePrompt, diff --git a/source/app/sections/interactive-app.spec.tsx b/source/app/sections/interactive-app.spec.tsx index 5884cc2be..2404ab8a8 100644 --- a/source/app/sections/interactive-app.spec.tsx +++ b/source/app/sections/interactive-app.spec.tsx @@ -1,6 +1,8 @@ import test from 'ava'; import {Text} from 'ink'; import React from 'react'; +import {DELAY_COMMAND_COMPLETE_MS} from '@/constants'; +import {useUserMessageQueue} from '@/hooks/useUserMessageQueue'; import stripAnsi from 'strip-ansi'; import type {Message} from '@/types'; import {renderWithTheme} from '../../test-utils/render-with-theme.js'; @@ -43,6 +45,13 @@ interface Overrides { setPendingPlanProceed?: (v: string | null) => void; handleMessageSubmit?: (message: string) => Promise; currentSessionId?: string | null; + toolManager?: unknown; + queuedMessages?: Array<{id: string; message: string; displayValue: string}>; + handleUserSubmit?: (message: string) => Promise; + drainNextMessage?: ( + dispatch: (message: {id: string; message: string; displayValue: string}) => + boolean | Promise, + ) => boolean | Promise; } function makeProps(o: Overrides = {}) { @@ -51,6 +60,7 @@ function makeProps(o: Overrides = {}) { const appState = { client: o.client ?? null, + toolManager: o.toolManager ?? null, messages: o.messages ?? [], currentModel: 'mock-model', currentProvider: 'mock', @@ -143,26 +153,240 @@ function makeProps(o: Overrides = {}) { pendingToolConfirmation: null, handleToolConfirmation: noop, handleQuestionAnswer: noop, - handleUserSubmit: noopAsync, + handleUserSubmit: o.handleUserSubmit ?? noopAsync, userMessageQueue: { - queuedMessages: [], + queuedMessages: o.queuedMessages ?? [], enqueueMessage: () => ({ id: 'queued-test', message: '', displayValue: '', }), removeMessage: noop, - drainNextMessage: () => false, + drainNextMessage: o.drainNextMessage ?? (async () => false), }, handleIdeSelect: noop, } as never; } +function QueuedPromptHarness({overrides}: {overrides: Overrides}) { + const userMessageQueue = useUserMessageQueue(); + + React.useEffect(() => { + userMessageQueue.enqueueMessage({ + message: 'queued prompt', + displayValue: 'queued prompt', + }); + }, [userMessageQueue.enqueueMessage]); + + return ( + + ); +} + test('renders without crashing in default state', t => { const {lastFrame} = renderWithTheme(); t.truthy(lastFrame()); }); +test('does not drain queued prompts while a turn is generating', async t => { + let dispatchAttempts = 0; + const {unmount} = renderWithTheme( + { + dispatchAttempts++; + }, + }} + />, + ); + + await new Promise(resolve => setTimeout(resolve, 25)); + t.is(dispatchAttempts, 0); + unmount(); +}); + +test('does not drain queued prompts while a modal mode is active', async t => { + let dispatchAttempts = 0; + const {unmount} = renderWithTheme( + { + dispatchAttempts++; + }, + }} + />, + ); + + await new Promise(resolve => setTimeout(resolve, 25)); + t.is(dispatchAttempts, 0); + unmount(); +}); + +test('does not drain queued prompts while plan review is active', async t => { + let dispatchAttempts = 0; + const {unmount} = renderWithTheme( + { + dispatchAttempts++; + }, + }} + />, + ); + + await new Promise(resolve => setTimeout(resolve, 25)); + t.is(dispatchAttempts, 0); + unmount(); +}); + +test('does not drain queued prompts while plan proceed is pending', async t => { + let dispatchAttempts = 0; + const {unmount} = renderWithTheme( + { + dispatchAttempts++; + }, + }} + />, + ); + + await new Promise(resolve => setTimeout(resolve, 25)); + t.is(dispatchAttempts, 0); + unmount(); +}); + +test('does not immediately retry a failed queued dispatch', async t => { + let dispatchAttempts = 0; + const {unmount} = renderWithTheme( + { + dispatchAttempts++; + throw new Error('dispatch failed'); + }, + }} + />, + ); + + await new Promise(resolve => setTimeout(resolve, 50)); + t.is(dispatchAttempts, 1); + unmount(); +}); + +test('drains every queued prompt after each dispatched turn returns to idle', async t => { + const submitted: string[] = []; + + const QueueDrainHarness = () => { + const userMessageQueue = useUserMessageQueue(); + const [isConversationComplete, setIsConversationComplete] = + React.useState(true); + + React.useEffect(() => { + userMessageQueue.enqueueMessage({message: 'first', displayValue: 'first'}); + userMessageQueue.enqueueMessage({message: 'second', displayValue: 'second'}); + }, [userMessageQueue.enqueueMessage]); + + return ( + { + submitted.push(message); + setIsConversationComplete(false); + await new Promise(resolve => setTimeout(resolve, 10)); + setIsConversationComplete(true); + }, + })} + userMessageQueue={userMessageQueue} + /> + ); + }; + + const {unmount} = renderWithTheme(); + await new Promise(resolve => setTimeout(resolve, 100)); + t.deepEqual(submitted, ['first', 'second']); + unmount(); +}); + +test('drains a prompt after delayed command completion when the app is idle', async t => { + const submitted: string[] = []; + + const DelayedCommandHarness = () => { + const userMessageQueue = useUserMessageQueue(); + const [isToolExecuting, setIsToolExecuting] = React.useState(true); + const [isConversationComplete, setIsConversationComplete] = + React.useState(false); + + React.useEffect(() => { + userMessageQueue.enqueueMessage({ + message: 'after compact', + displayValue: 'after compact', + }); + const timeout = setTimeout(() => { + setIsToolExecuting(false); + setIsConversationComplete(true); + }, DELAY_COMMAND_COMPLETE_MS); + + return () => clearTimeout(timeout); + }, [userMessageQueue.enqueueMessage]); + + return ( + { + submitted.push(message); + }, + })} + userMessageQueue={userMessageQueue} + /> + ); + }; + + const {unmount} = renderWithTheme(); + await new Promise(resolve => + setTimeout(resolve, DELAY_COMMAND_COMPLETE_MS + 40), + ); + t.deepEqual(submitted, ['after compact']); + unmount(); +}); + test('renders the static-component marker through ChatHistory', t => { const {lastFrame} = renderWithTheme( , diff --git a/source/app/sections/interactive-app.tsx b/source/app/sections/interactive-app.tsx index 965567ad0..b0073218c 100644 --- a/source/app/sections/interactive-app.tsx +++ b/source/app/sections/interactive-app.tsx @@ -101,6 +101,9 @@ export function InteractiveApp({ React.useState(null); const [restoredDraft, setRestoredDraft] = React.useState(null); + const drainInProgressRef = React.useRef(false); + const lastFailedDrainIdRef = React.useRef(null); + const [drainAttempt, setDrainAttempt] = React.useState(0); const handleToggleCompactDisplay = () => { const expanding = appState.compactToolDisplay; @@ -183,6 +186,99 @@ export function InteractiveApp({ appState.abortController !== null) && !appState.liveComponentCapturesInput; + // Drain queued prompts only after the previous turn is fully idle and all + // modal modes have closed. Command handlers and conversation completion can + // both signal completion, so keeping the drain here makes it idempotent and + // prevents nested or duplicate turns. + const queueDrainBlocked = + appState.isCancelling || + chatHandler.isGenerating || + appState.isToolExecuting || + appState.abortController !== null || + appState.isToolConfirmationMode || + appState.isQuestionMode || + pendingSubagentApproval !== null || + pendingToolConfirmation !== null || + appState.planReviewState?.show === true || + appState.pendingPlanProceed !== null; + const queuedMessageCount = userMessageQueue.queuedMessages.length; + const queuedMessageId = userMessageQueue.queuedMessages[0]?.id; + + React.useEffect(() => { + // Re-run after a successful dispatch settles, once its queue update has + // rendered and the next item can be considered. + void drainAttempt; + if ( + queueDrainBlocked || + appState.activeMode !== null || + appState.isSettingsMode || + !appState.client || + !appState.toolManager || + !appState.isConversationComplete || + queuedMessageCount === 0 || + lastFailedDrainIdRef.current === queuedMessageId || + drainInProgressRef.current + ) { + return; + } + + drainInProgressRef.current = true; + let started = false; + const timeout = setTimeout(() => { + started = true; + let drainedMessageId = queuedMessageId ?? null; + void Promise.resolve() + .then(() => + userMessageQueue.drainNextMessage(async message => { + drainedMessageId = message.id; + await handleUserSubmit( + message.message, + message.displayValue, + message.images, + ); + return true; + }), + ) + .then( + dispatched => { + drainInProgressRef.current = false; + if (!dispatched) { + // Keep a failed head queued, but do not immediately re-enter + // the effect while it still has the same identity. + lastFailedDrainIdRef.current = drainedMessageId; + return; + } + lastFailedDrainIdRef.current = null; + // The queue state update happens before the dispatch resolves. A + // separate render is needed to notice and drain the next item after + // the dispatched turn returns to idle. + setDrainAttempt(attempt => attempt + 1); + }, + () => { + drainInProgressRef.current = false; + lastFailedDrainIdRef.current = drainedMessageId; + }, + ); + }, 0); + + return () => { + clearTimeout(timeout); + if (!started) drainInProgressRef.current = false; + }; + }, [ + appState.activeMode, + appState.client, + appState.isConversationComplete, + appState.isSettingsMode, + appState.toolManager, + queueDrainBlocked, + handleUserSubmit, + userMessageQueue.drainNextMessage, + queuedMessageCount, + queuedMessageId, + drainAttempt, + ]); + const recallableSubmittedDraft = cancellable && chatHandler.isGenerating && diff --git a/source/app/utils/app-util.spec.ts b/source/app/utils/app-util.spec.ts index 4942ee24e..05d5a5415 100644 --- a/source/app/utils/app-util.spec.ts +++ b/source/app/utils/app-util.spec.ts @@ -431,6 +431,22 @@ test.serial('chat message - displayValue is optional (callers without a placehol t.is(received.displayValue, undefined); }); +test.serial('delayed slash-command completion is delivered after the handler returns', async t => { + let completed = false; + const options = createResumeTestOptions({ + onCommandComplete: () => { + completed = true; + }, + }); + options.onShowStatus = () => {}; + + await handleMessageSubmission('/status', options); + + t.false(completed); + await new Promise(resolve => setTimeout(resolve, 125)); + t.true(completed); +}); + test.serial('retry command - /retry without a prior user turn shows an error', async t => { let queued: React.ReactNode = null; let submitted = false; diff --git a/source/app/utils/handlers/retry-handler.spec.ts b/source/app/utils/handlers/retry-handler.spec.ts new file mode 100644 index 000000000..66895dda2 --- /dev/null +++ b/source/app/utils/handlers/retry-handler.spec.ts @@ -0,0 +1,24 @@ +import test from 'ava'; +import type {MessageSubmissionOptions} from '@/types'; +import {handleRetryCommand} from './retry-handler.js'; + +test('does not signal command completion after the retried turn returns', async t => { + let chatCalls = 0; + let completionCalls = 0; + + const options = { + messages: [{role: 'user', content: 'retry me'}], + provider: 'mock', + onAddToChatQueue: () => {}, + onHandleChatMessage: async () => { + chatCalls++; + }, + onCommandComplete: () => { + completionCalls++; + }, + } as unknown as MessageSubmissionOptions; + + t.true(await handleRetryCommand(['retry'], options)); + t.is(chatCalls, 1); + t.is(completionCalls, 0); +}); diff --git a/source/app/utils/handlers/retry-handler.ts b/source/app/utils/handlers/retry-handler.ts index 6bf654f33..328d75607 100644 --- a/source/app/utils/handlers/retry-handler.ts +++ b/source/app/utils/handlers/retry-handler.ts @@ -86,6 +86,7 @@ export async function handleRetryCommand( lastUserMessage.content, lastUserMessage.content, ); - options.onCommandComplete?.(); + // The retried chat turn owns its completion signal. Emitting another one + // here can start the next queued prompt while that turn is still unwinding. return true; } diff --git a/source/components/user-input.spec.tsx b/source/components/user-input.spec.tsx index c3b553d63..0d9ba7719 100644 --- a/source/components/user-input.spec.tsx +++ b/source/components/user-input.spec.tsx @@ -420,7 +420,37 @@ test('UserInput navigates queued messages while busy with empty input', async t unmount(); }); -test('UserInput loads selected queued message for editing', async t => { +test('UserInput loads selected queued message for editing while idle', async t => { + let removedId = ''; + + const {stdin, lastFrame, unmount} = render( + + { + removedId = id; + }} + /> + , + ); + + stdin.write('\u001B[B'); + await wait(50); + stdin.write('\u001B[B'); + await wait(50); + stdin.write('\r'); + await wait(50); + + t.is(removedId, 'queued-2'); + t.regex(lastFrame()!, /second queued/); + unmount(); +}); + +test('UserInput loads selected queued message for editing while busy', async t => { let removedId = ''; const {stdin, lastFrame, unmount} = render( @@ -1163,4 +1193,4 @@ test.serial('UserInput ignores terminal pastes while disabled', async t => { t.notRegex(lastFrame()!, /should not appear/); unmount(); }); - + diff --git a/source/components/user-input.tsx b/source/components/user-input.tsx index 64df076f7..66fc96ba9 100644 --- a/source/components/user-input.tsx +++ b/source/components/user-input.tsx @@ -633,7 +633,7 @@ export default function UserInput({ const handleQueueNavigation = useCallback( (direction: 'up' | 'down') => { - if (!isBusy || input.length > 0 || queuedMessages.length === 0) { + if (input.length > 0 || queuedMessages.length === 0) { return false; } @@ -656,12 +656,11 @@ export default function UserInput({ setSelectedQueuedIndex(selectedQueuedIndex + 1); return true; }, - [isBusy, input.length, queuedMessages.length, selectedQueuedIndex], + [input.length, queuedMessages.length, selectedQueuedIndex], ); const loadSelectedQueuedMessage = useCallback(() => { if ( - !isBusy || input.length > 0 || selectedQueuedIndex < 0 || selectedQueuedIndex >= queuedMessages.length @@ -682,7 +681,6 @@ export default function UserInput({ setTextInputKey(prev => prev + 1); return true; }, [ - isBusy, input.length, selectedQueuedIndex, queuedMessages, @@ -692,7 +690,6 @@ export default function UserInput({ const removeSelectedQueuedMessage = useCallback(() => { if ( - !isBusy || input.length > 0 || selectedQueuedIndex < 0 || selectedQueuedIndex >= queuedMessages.length @@ -706,7 +703,6 @@ export default function UserInput({ ); return true; }, [ - isBusy, input.length, selectedQueuedIndex, queuedMessages, diff --git a/source/hooks/chat-handler/useChatHandler.spec.tsx b/source/hooks/chat-handler/useChatHandler.spec.tsx index c20a74e3d..26a2d66f6 100644 --- a/source/hooks/chat-handler/useChatHandler.spec.tsx +++ b/source/hooks/chat-handler/useChatHandler.spec.tsx @@ -293,6 +293,30 @@ test('useChatHandler - handles null client gracefully', t => { t.truthy(hookResult); }); +test('useChatHandler - signals completion when chat dependencies are unavailable', async t => { + let hookResult: ChatHandlerReturn | null = null; + let completionCalls = 0; + + const rendered = render( + { + completionCalls++; + }, + })} + onResult={result => { + hookResult = result; + }} + />, + ); + + await waitForCondition(() => hookResult !== null); + await hookResult!.handleChatMessage('queued after unavailable setup'); + + t.is(completionCalls, 1); + rendered.unmount(); +}); + test('useChatHandler - setMessages callback works', t => { let hookResult: ChatHandlerReturn | null = null; diff --git a/source/hooks/chat-handler/useChatHandler.tsx b/source/hooks/chat-handler/useChatHandler.tsx index 5f933a7bd..5d78aba62 100644 --- a/source/hooks/chat-handler/useChatHandler.tsx +++ b/source/hooks/chat-handler/useChatHandler.tsx @@ -343,7 +343,13 @@ export function useChatHandler({ displayValue?: string, images?: ImageAttachment[], ) => { - if (!client || !toolManager) return; + if (!client || !toolManager) { + // handleMessageSubmit marks a turn as incomplete before reaching this + // hook. Signal completion here as well so an unavailable setup cannot + // leave the queue blocked forever. + onConversationComplete?.(); + return; + } const sessionId = ensureCurrentSessionId?.(); let wrotePlan = false; let finalAssistantText = ''; diff --git a/source/hooks/useAppHandlers.spec.tsx b/source/hooks/useAppHandlers.spec.tsx index 830c87564..8d8de005d 100644 --- a/source/hooks/useAppHandlers.spec.tsx +++ b/source/hooks/useAppHandlers.spec.tsx @@ -198,6 +198,14 @@ test('returns the expected handler surface', t => { t.is(typeof handlers.handleMessageSubmit, 'function'); }); +test('signals slash-command completion so queued work can resume', async t => { + const {handlers, spies} = setup(); + + await handlers.handleMessageSubmit('/compact'); + + t.deepEqual(spies.setIsConversationComplete.calls, [[false], [true]]); +}); + test('handleCancel without an abort controller is a no-op', t => { const { handlers, spies } = setup({ abortController: null }); @@ -239,6 +247,7 @@ test('declining execution keeps Plan Mode active and asks for revisions', t => { handlers.handlePlanModify(); + t.deepEqual(spies.setIsConversationComplete.calls, [[false]]); t.deepEqual(spies.setPlanReviewState.calls, [[null]]); t.deepEqual(spies.setDevelopmentMode.calls, []); const notice = spies.addToChatQueue.calls.at(-1)?.[0]; @@ -256,6 +265,18 @@ test('declining execution keeps Plan Mode active and asks for revisions', t => { ); }); +test('asking for clarification blocks queued prompts until the turn starts', async t => { + const {handlers, spies} = setup({developmentMode: 'plan'}); + + await handlers.handlePlanAskMore(); + + t.deepEqual(spies.setIsConversationComplete.calls, [[false]]); + t.deepEqual(spies.setPlanReviewState.calls, [[null]]); + t.deepEqual(spies.handleChatMessage.calls, [ + ['please ask me any additional clarifying questions before proceeding'], + ]); +}); + async function withMockConfig( config: any, preferences: any, diff --git a/source/hooks/useAppHandlers.tsx b/source/hooks/useAppHandlers.tsx index ec594db2e..bb2de009e 100644 --- a/source/hooks/useAppHandlers.tsx +++ b/source/hooks/useAppHandlers.tsx @@ -645,6 +645,7 @@ export function useAppHandlers(props: UseAppHandlersProps): AppHandlers { const handlePlanAskMore = React.useCallback(async () => { // Hide the review bar and stay in plan mode; the model asks its questions // and the user answers before a new plan is produced. + props.setIsConversationComplete(false); props.setPlanReviewState(null); await props.handleChatMessage( 'please ask me any additional clarifying questions before proceeding', @@ -653,6 +654,8 @@ export function useAppHandlers(props: UseAppHandlersProps): AppHandlers { const handlePlanModify = React.useCallback(() => { // Return to input without changing mode so the user can request revisions. + // Keep the queue blocked until that revision turn has completed. + props.setIsConversationComplete(false); props.setPlanReviewState(null); props.addToChatQueue(