From 041343344cf13b578450809f4fb1e1a9ccb6378b Mon Sep 17 00:00:00 2001 From: kavish-19 <63698788+kavish-19@users.noreply.github.com> Date: Thu, 3 Sep 2026 19:50:58 +0530 Subject: [PATCH 1/2] fix(cli): queue /plan, /interview, /review mid-turn instead of interrupting /plan , /interview , and /review (and their input-mode counterparts when submitted without inline args) called sendMessage() directly with no check for whether a run was already in progress. Firing one of these while a previous message was still streaming registered a new active-run owner, which force-stops the in-flight run ('user-interrupt') instead of queuing behind it -- so the current job was interrupted and lost rather than queued, matching what #1211 describes. /skill: already gets this right via dispatchSkillPrompt, which checks isStreaming/streamMessageIdRef/isChainInProgressRef and falls back to addToQueue when busy. Extract that logic into a shared sendOrQueuePrompt() helper and route all six call sites (three in command-registry.ts, three in router.ts) through it so they can't drift out of sync with the busy check again. Added a failing-first regression test covering both entry paths (input mode and inline slash-command args) for all three commands, confirmed red against the unfixed code, green after. Claude-Session: https://claude.ai/code/session_018vPhyqaaoKa8cgs7GEnyq5 --- .../__tests__/router-steering.test.ts | 95 ++++++++++++++++++- cli/src/commands/command-registry.ts | 70 +++++++------- cli/src/commands/router.ts | 16 +--- 3 files changed, 134 insertions(+), 47 deletions(-) diff --git a/cli/src/commands/__tests__/router-steering.test.ts b/cli/src/commands/__tests__/router-steering.test.ts index cb15dcf92b..cd99c6b566 100644 --- a/cli/src/commands/__tests__/router-steering.test.ts +++ b/cli/src/commands/__tests__/router-steering.test.ts @@ -6,11 +6,14 @@ import { activateSteering, drainSteeringMessages, } from '../../utils/steering-buffer' +import { findCommand } from '../command-registry' import { routeUserPrompt } from '../router' import type { RouterParams } from '../command-registry' -const createMockParams = (overrides: Partial = {}): RouterParams => +const createMockParams = ( + overrides: Partial = {}, +): RouterParams => ({ agentMode: 'DEFAULT', inputRef: { current: null }, @@ -125,4 +128,94 @@ describe('mid-turn routing', () => { expect(params.sendMessage).toHaveBeenCalledTimes(1) expect(drainSteeringMessages('run-1')).toEqual([]) }) + + describe('plan/interview/review input modes queue instead of interrupting', () => { + afterEach(() => { + useChatStore.getState().setInputMode('default') + }) + + test('plan mode queues a mid-turn submit instead of sending it', async () => { + useChatStore.getState().setInputMode('plan') + const params = createMockParams({ + inputValue: 'add dark mode', + isStreaming: true, + }) + await routeUserPrompt(params) + + // Must never fire a second run against the busy owner: that would + // register a new active-run owner and interrupt the in-flight one. + expect(params.sendMessage).not.toHaveBeenCalled() + expect(params.addToQueue).toHaveBeenCalledTimes(1) + const [queued] = (params.addToQueue as ReturnType).mock + .calls[0] as [string] + expect(queued).toContain('add dark mode') + }) + + test('interview mode queues a mid-turn submit instead of sending it', async () => { + useChatStore.getState().setInputMode('interview') + const params = createMockParams({ + inputValue: 'what should the API look like', + isStreaming: true, + }) + await routeUserPrompt(params) + + expect(params.sendMessage).not.toHaveBeenCalled() + expect(params.addToQueue).toHaveBeenCalledTimes(1) + }) + + test('review mode queues a mid-turn submit instead of sending it', async () => { + useChatStore.getState().setInputMode('review') + const params = createMockParams({ + inputValue: 'check for null handling', + isStreaming: true, + }) + await routeUserPrompt(params) + + expect(params.sendMessage).not.toHaveBeenCalled() + expect(params.addToQueue).toHaveBeenCalledTimes(1) + }) + + test('plan mode still sends immediately when idle', async () => { + useChatStore.getState().setInputMode('plan') + const params = createMockParams({ inputValue: 'add dark mode' }) + await routeUserPrompt(params) + + expect(params.sendMessage).toHaveBeenCalledTimes(1) + expect(params.addToQueue).not.toHaveBeenCalled() + }) + }) + + describe('/interview and /review with inline args queue instead of interrupting', () => { + test('/interview queues mid-turn instead of sending', () => { + const params = createMockParams({ + inputValue: '/interview what should the API look like', + isStreaming: true, + }) + findCommand('interview')!.handler(params, 'what should the API look like') + + expect(params.sendMessage).not.toHaveBeenCalled() + expect(params.addToQueue).toHaveBeenCalledTimes(1) + }) + + test('/review queues mid-turn instead of sending', () => { + const params = createMockParams({ + inputValue: '/review check for null handling', + isStreaming: true, + }) + findCommand('review')!.handler(params, 'check for null handling') + + expect(params.sendMessage).not.toHaveBeenCalled() + expect(params.addToQueue).toHaveBeenCalledTimes(1) + }) + + test('/interview still sends immediately when idle', () => { + const params = createMockParams({ + inputValue: '/interview what should the API look like', + }) + findCommand('interview')!.handler(params, 'what should the API look like') + + expect(params.sendMessage).toHaveBeenCalledTimes(1) + expect(params.addToQueue).not.toHaveBeenCalled() + }) + }) }) diff --git a/cli/src/commands/command-registry.ts b/cli/src/commands/command-registry.ts index 9c381796fc..ecb482c39c 100644 --- a/cli/src/commands/command-registry.ts +++ b/cli/src/commands/command-registry.ts @@ -608,15 +608,10 @@ const ALL_COMMANDS: CommandDefinition[] = [ params.saveToHistory(params.inputValue.trim()) clearInput(params) - // If user provided text directly, send it immediately + // If user provided text directly, send it now (or queue it if a run + // is already in progress) if (trimmedArgs) { - params.sendMessage({ - content: buildInterviewPrompt(trimmedArgs), - agentMode: params.agentMode, - }) - setTimeout(() => { - params.scrollToLatest() - }, 0) + sendOrQueuePrompt(params, buildInterviewPrompt(trimmedArgs)) return } @@ -633,15 +628,10 @@ const ALL_COMMANDS: CommandDefinition[] = [ params.saveToHistory(params.inputValue.trim()) clearInput(params) - // If user provided plan text directly, send it immediately + // If user provided plan text directly, send it now (or queue it if a + // run is already in progress) if (trimmedArgs) { - params.sendMessage({ - content: buildPlanPrompt(trimmedArgs), - agentMode: params.agentMode, - }) - setTimeout(() => { - params.scrollToLatest() - }, 0) + sendOrQueuePrompt(params, buildPlanPrompt(trimmedArgs)) return } @@ -658,15 +648,10 @@ const ALL_COMMANDS: CommandDefinition[] = [ params.saveToHistory(params.inputValue.trim()) clearInput(params) - // If user provided review text directly, send it immediately without showing the screen + // If user provided review text directly, send it now without showing + // the screen (or queue it if a run is already in progress) if (trimmedArgs) { - params.sendMessage({ - content: buildReviewPromptFromArgs(trimmedArgs), - agentMode: params.agentMode, - }) - setTimeout(() => { - params.scrollToLatest() - }, 0) + sendOrQueuePrompt(params, buildReviewPromptFromArgs(trimmedArgs)) return } @@ -808,33 +793,50 @@ function createSkillCommand(skillName: string): CommandDefinition { } /** - * Send (or queue, mid-turn) a user-invoked skill prompt. Shared by the - * /skill: args form and the skill input mode's submit (router), so the - * two entry paths for the same feature cannot drift. + * Send a prompt immediately, or queue it behind an in-progress run so it + * isn't dropped. Shared by every command whose handler can also fire + * mid-turn (skill args, /plan, /interview, /review, and their input-mode + * counterparts in the router) so those entry paths can't drift out of sync + * with the busy check. */ -export function dispatchSkillPrompt( +export function sendOrQueuePrompt( params: RouterParams, - skill: { name: string; content: string }, - input: string, + content: string, + attachments: PendingAttachment[] = [], ): void { - const userPrompt = buildSkillPrompt(skill, input) - if ( params.isStreaming || params.streamMessageIdRef.current || params.isChainInProgressRef.current ) { - params.addToQueue(userPrompt, capturePendingAttachments()) + params.addToQueue(content, attachments) params.setInputFocused(true) params.inputRef.current?.focus() return } params.sendMessage({ - content: userPrompt, + content, agentMode: params.agentMode, }) setTimeout(() => { params.scrollToLatest() }, 0) } + +/** + * Send (or queue, mid-turn) a user-invoked skill prompt. Shared by the + * /skill: args form and the skill input mode's submit (router), so the + * two entry paths for the same feature cannot drift. + */ +export function dispatchSkillPrompt( + params: RouterParams, + skill: { name: string; content: string }, + input: string, +): void { + sendOrQueuePrompt( + params, + buildSkillPrompt(skill, input), + capturePendingAttachments(), + ) +} diff --git a/cli/src/commands/router.ts b/cli/src/commands/router.ts index 7453034ebe..a588cf56c7 100644 --- a/cli/src/commands/router.ts +++ b/cli/src/commands/router.ts @@ -5,6 +5,7 @@ import { runTerminalCommand } from '@codebuff/sdk' import { dispatchSkillPrompt, findCommand, + sendOrQueuePrompt, type RouterParams, type CommandResult, } from './command-registry' @@ -325,10 +326,7 @@ export async function routeUserPrompt( setInputFocused(true) inputRef.current?.focus() - sendMessage({ content: buildPlanPrompt(trimmed), agentMode }) - setTimeout(() => { - scrollToLatest() - }, 0) + sendOrQueuePrompt(params, buildPlanPrompt(trimmed)) return } @@ -341,10 +339,7 @@ export async function routeUserPrompt( setInputFocused(true) inputRef.current?.focus() - sendMessage({ content: buildInterviewPrompt(trimmed), agentMode }) - setTimeout(() => { - scrollToLatest() - }, 0) + sendOrQueuePrompt(params, buildInterviewPrompt(trimmed)) return } @@ -389,10 +384,7 @@ export async function routeUserPrompt( setInputFocused(true) inputRef.current?.focus() - sendMessage({ content: buildReviewPrompt('custom', trimmed), agentMode }) - setTimeout(() => { - scrollToLatest() - }, 0) + sendOrQueuePrompt(params, buildReviewPrompt('custom', trimmed)) return } From dffc4bff7476d039a74155e685b9a5d2052f7ead Mon Sep 17 00:00:00 2001 From: kavish-19 <63698788+kavish-19@users.noreply.github.com> Date: Fri, 4 Sep 2026 22:49:01 +0530 Subject: [PATCH 2/2] fix(cli): keep staged attachments with the prompt they were staged for Review feedback on #1256 asked whether plan/interview/review needed to capture pending attachments. Checking it turned up two real defects, one of them introduced by that PR. prepareUserMessage resolves attachments as `attachments ?? useChatStore.getState().pendingAttachments`, so passing an explicit array suppresses the store fallback and passing no key at all uses it. sendOrQueuePrompt got that backwards on both branches: - The queue branch defaulted to `[]`, so a mid-turn /plan, /interview or /review queued with no attachments and left the staged ones in the store, where they attached to whatever the user sent next. Before #1256 these paths called sendMessage with no attachments key and picked them up via the fallback, so this was a regression, not a pre-existing gap. - dispatchSkillPrompt passed capturePendingAttachments() as an argument, which evaluates before the busy check. An idle /skill: therefore cleared the store and then sent without the captured value, dropping the attachments outright. Capture inside the queue branch instead, where the skill path already had it, and drop the parameter so neither call site can reintroduce the split. Both defects are covered by tests that fail against the previous commit. Claude-Session: https://claude.ai/code/session_018vPhyqaaoKa8cgs7GEnyq5 --- .../__tests__/router-steering.test.ts | 44 ++++++++++++++++++- cli/src/commands/command-registry.ts | 22 +++++----- 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/cli/src/commands/__tests__/router-steering.test.ts b/cli/src/commands/__tests__/router-steering.test.ts index cd99c6b566..8ab90d84cc 100644 --- a/cli/src/commands/__tests__/router-steering.test.ts +++ b/cli/src/commands/__tests__/router-steering.test.ts @@ -6,7 +6,7 @@ import { activateSteering, drainSteeringMessages, } from '../../utils/steering-buffer' -import { findCommand } from '../command-registry' +import { dispatchSkillPrompt, findCommand } from '../command-registry' import { routeUserPrompt } from '../router' import type { RouterParams } from '../command-registry' @@ -39,11 +39,13 @@ const createMockParams = ( beforeEach(() => { useChatStore.getState().clearPendingBashMessages() + useChatStore.getState().clearPendingAttachments() }) afterEach(() => { __resetSteeringForTests() useChatStore.getState().clearPendingBashMessages() + useChatStore.getState().clearPendingAttachments() }) describe('mid-turn routing', () => { @@ -218,4 +220,44 @@ describe('mid-turn routing', () => { expect(params.addToQueue).not.toHaveBeenCalled() }) }) + + describe('staged attachments follow the prompt they were staged for', () => { + const stageAttachment = () => + useChatStore.getState().addPendingAttachment({ + kind: 'text', + id: 'pasted-1', + content: 'a long pasted block', + preview: 'a long pasted block', + charCount: 19, + }) + + test('a queued /interview carries the staged attachments with it', () => { + stageAttachment() + const params = createMockParams({ + inputValue: '/interview what should the API look like', + isStreaming: true, + }) + findCommand('interview')!.handler(params, 'what should the API look like') + + const [, attachments] = (params.addToQueue as ReturnType) + .mock.calls[0] as [string, unknown[]] + // Queued sends pass their attachments explicitly, which suppresses the + // pendingAttachments fallback in prepareUserMessage. Anything left in + // the store here would land on some later, unrelated message. + expect(attachments).toHaveLength(1) + expect(useChatStore.getState().pendingAttachments).toHaveLength(0) + }) + + test('an idle skill dispatch leaves the staged attachments for the send path', () => { + stageAttachment() + const params = createMockParams({ inputValue: '/skill:tidy' }) + dispatchSkillPrompt(params, { name: 'tidy', content: 'Tidy up.' }, '') + + expect(params.sendMessage).toHaveBeenCalledTimes(1) + // sendMessage is called without an attachments key on purpose: it falls + // back to the store. Capturing here would clear them into a value the + // idle branch never passes on. + expect(useChatStore.getState().pendingAttachments).toHaveLength(1) + }) + }) }) diff --git a/cli/src/commands/command-registry.ts b/cli/src/commands/command-registry.ts index ecb482c39c..5eb3b157b5 100644 --- a/cli/src/commands/command-registry.ts +++ b/cli/src/commands/command-registry.ts @@ -798,18 +798,22 @@ function createSkillCommand(skillName: string): CommandDefinition { * mid-turn (skill args, /plan, /interview, /review, and their input-mode * counterparts in the router) so those entry paths can't drift out of sync * with the busy check. + * + * Attachments are handled on exactly one side of the branch, and must stay + * that way. A queued send passes its attachments explicitly, which suppresses + * the pendingAttachments fallback in prepareUserMessage, so the queue entry + * has to carry them or they sit in the store and surface on some later, + * unrelated message. An immediate send passes no attachments key at all and + * relies on that same fallback, so capturing here would clear them into a + * value this branch never forwards. */ -export function sendOrQueuePrompt( - params: RouterParams, - content: string, - attachments: PendingAttachment[] = [], -): void { +export function sendOrQueuePrompt(params: RouterParams, content: string): void { if ( params.isStreaming || params.streamMessageIdRef.current || params.isChainInProgressRef.current ) { - params.addToQueue(content, attachments) + params.addToQueue(content, capturePendingAttachments()) params.setInputFocused(true) params.inputRef.current?.focus() return @@ -834,9 +838,5 @@ export function dispatchSkillPrompt( skill: { name: string; content: string }, input: string, ): void { - sendOrQueuePrompt( - params, - buildSkillPrompt(skill, input), - capturePendingAttachments(), - ) + sendOrQueuePrompt(params, buildSkillPrompt(skill, input)) }