From e209c7c81c68c2ad4422c2595329efa2e600f613 Mon Sep 17 00:00:00 2001 From: Roomote Date: Tue, 28 Jul 2026 22:55:10 +0000 Subject: [PATCH 1/5] fix(terminal): delay command_output ask for short foreground commands The command_output ask fired on the first output chunk of any foreground command, prompting users even for commands about to complete. Schedule the ask instead so it only fires when the command is still running after a 5s delay, preserving the interrupt/feedback path for long-running commands while letting short commands finish without prompting. Closes #1042 --- src/core/tools/ExecuteCommandTool.ts | 80 ++++++++++--- .../__tests__/executeCommandTool.spec.ts | 106 ++++++++++++++++++ 2 files changed, 168 insertions(+), 18 deletions(-) diff --git a/src/core/tools/ExecuteCommandTool.ts b/src/core/tools/ExecuteCommandTool.ts index 3d6a44ef18..b2858433f1 100644 --- a/src/core/tools/ExecuteCommandTool.ts +++ b/src/core/tools/ExecuteCommandTool.ts @@ -35,6 +35,15 @@ export function canRetryShellIntegrationError(error: unknown): error is ShellInt return error instanceof ShellIntegrationError && !error.commandSubmitted } +/** + * Grace period before a foreground command may trigger a `command_output` ask. + * Short commands that emit output and exit within this window never prompt the + * user; the ask only fires when the command is still running once the delay + * elapses, so users can still interrupt or provide feedback on long-running + * commands. + */ +export const COMMAND_OUTPUT_ASK_DELAY_MS = 5_000 + export function getTerminalProviderForExecution(terminalShellIntegrationDisabled: boolean): { terminalProvider: RooTerminalProvider isCmdExeFallback: boolean @@ -340,6 +349,51 @@ export async function executeCommandInTerminal( resolveOnCompleted = resolve }) + // Delay the `command_output` ask so short foreground commands that emit + // output and exit normally never prompt the user. The ask only fires if the + // command is still running once COMMAND_OUTPUT_ASK_DELAY_MS has elapsed + // since execution started, preserving the interrupt/feedback path for + // long-running commands. + let commandStartedAt = 0 + let commandOutputAskTimer: NodeJS.Timeout | undefined + + const askForCommandOutput = async (process: RooTerminalProcess): Promise => { + if (runInBackground || hasAskedForCommandOutput || completed) { + return + } + + // Mark that we've asked to prevent multiple concurrent asks + hasAskedForCommandOutput = true + + try { + const { response, text, images } = await task.ask("command_output", "") + runInBackground = true + + if (response === "messageResponse") { + message = { text, images } + process.continue() + } + } catch (_error) { + // Silently handle ask errors (e.g., "Current ask promise was ignored") + } + } + + const scheduleCommandOutputAsk = (process: RooTerminalProcess): void => { + if (runInBackground || hasAskedForCommandOutput || completed || commandOutputAskTimer) { + return + } + + const remainingDelay = COMMAND_OUTPUT_ASK_DELAY_MS - (Date.now() - commandStartedAt) + + commandOutputAskTimer = setTimeout( + () => { + commandOutputAskTimer = undefined + void askForCommandOutput(process) + }, + Math.max(remainingDelay, 0), + ) + } + const callbacks: RooTerminalCallbacks = { onLine: async (lines: string, process: RooTerminalProcess) => { accumulatedOutput += lines @@ -359,26 +413,12 @@ export async function executeCommandInTerminal( provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) }) schedulePartialCommandOutputUpdate() - if (runInBackground || hasAskedForCommandOutput) { - return - } - - // Mark that we've asked to prevent multiple concurrent asks - hasAskedForCommandOutput = true - - try { - const { response, text, images } = await task.ask("command_output", "") - runInBackground = true - - if (response === "messageResponse") { - message = { text, images } - process.continue() - } - } catch (_error) { - // Silently handle ask errors (e.g., "Current ask promise was ignored") - } + scheduleCommandOutputAsk(process) }, onCompleted: async (output: string | undefined) => { + clearTimeout(commandOutputAskTimer) + commandOutputAskTimer = undefined + clearTimeout(pendingCommandOutputEmitTimer) pendingCommandOutputEmitTimer = undefined @@ -441,6 +481,7 @@ export async function executeCommandInTerminal( workingDir = terminal.getCurrentWorkingDirectory() } + commandStartedAt = Date.now() const process = terminal.runCommand(command, callbacks) task.terminalProcess = process @@ -462,6 +503,8 @@ export async function executeCommandInTerminal( new Promise((resolve) => { agentTimeoutId = setTimeout(() => { runInBackground = true + clearTimeout(commandOutputAskTimer) + commandOutputAskTimer = undefined process.continue() task.supersedePendingAsk() resolve() @@ -501,6 +544,7 @@ export async function executeCommandInTerminal( } finally { clearTimeout(agentTimeoutId) clearTimeout(userTimeoutId) + clearTimeout(commandOutputAskTimer) clearTimeout(pendingCommandOutputEmitTimer) task.terminalProcess = undefined } diff --git a/src/core/tools/__tests__/executeCommandTool.spec.ts b/src/core/tools/__tests__/executeCommandTool.spec.ts index 1725154328..ea412e0cd5 100644 --- a/src/core/tools/__tests__/executeCommandTool.spec.ts +++ b/src/core/tools/__tests__/executeCommandTool.spec.ts @@ -8,6 +8,7 @@ import { formatResponse } from "../../prompts/responses" import { ToolUse, AskApproval, HandleError, PushToolResult } from "../../../shared/tools" import { unescapeHtmlEntities } from "../../../utils/text-normalization" import { Terminal } from "../../../integrations/terminal/Terminal" +import type { RooTerminalCallbacks, RooTerminalProcess } from "../../../integrations/terminal/types" // Mock dependencies vitest.mock("execa", () => ({ @@ -333,4 +334,109 @@ describe("executeCommandTool", () => { expect(executeCommandModule.resolveAgentTimeoutMs(30)).toBe(30_000) }) }) + + describe("command_output ask policy", () => { + type MockProcess = Promise & { + continue: ReturnType + abort: ReturnType + } + + interface ControllableTerminal { + callbacks: RooTerminalCallbacks | undefined + proc: MockProcess + resolveProcess: () => void + } + + const setupControllableTerminal = async (): Promise => { + const { TerminalRegistry } = await import("../../../integrations/terminal/TerminalRegistry") + const state: ControllableTerminal = { + callbacks: undefined, + proc: undefined as unknown as MockProcess, + resolveProcess: () => {}, + } + const processPromise = new Promise((resolve) => { + state.resolveProcess = resolve + }) + state.proc = Object.assign(processPromise, { continue: vitest.fn(), abort: vitest.fn() }) + ;(TerminalRegistry.getOrCreateTerminal as ReturnType).mockResolvedValue({ + runCommand: vitest.fn((_cmd: string, callbacks: RooTerminalCallbacks) => { + state.callbacks = callbacks + return state.proc + }), + getCurrentWorkingDirectory: vitest.fn().mockReturnValue("/test/workspace"), + }) + return state + } + + const handleCommand = (command: string) => { + mockToolUse.params.command = command + mockToolUse.nativeArgs = { command } + + return executeCommandTool.handle(mockCline as unknown as Task, mockToolUse, { + askApproval: mockAskApproval as unknown as AskApproval, + handleError: mockHandleError as unknown as HandleError, + pushToolResult: mockPushToolResult as unknown as PushToolResult, + }) + } + + it("does not ask about command output when a short command emits output and exits normally", async () => { + vitest.useFakeTimers() + const terminal = await setupControllableTerminal() + + const handlePromise = handleCommand("echo hello") + + await vitest.waitFor(() => expect(terminal.callbacks).toBeDefined()) + const callbacks = terminal.callbacks! + const proc = terminal.proc as unknown as RooTerminalProcess + + await callbacks.onLine("hello\n", proc) + await callbacks.onCompleted!("hello\n", proc) + callbacks.onShellExecutionComplete!({ exitCode: 0 }, proc) + terminal.resolveProcess() + + // Advance past the ask delay to prove the scheduled ask was cancelled + // on completion, not merely deferred beyond the test's runtime. + await vitest.advanceTimersByTimeAsync(executeCommandModule.COMMAND_OUTPUT_ASK_DELAY_MS + 1_000) + + await handlePromise + + expect(mockCline.ask).not.toHaveBeenCalled() + expect(mockPushToolResult).toHaveBeenCalled() + const result = mockPushToolResult.mock.calls[0][0] + expect(result).toContain("hello") + expect(result).toContain("Exit code: 0") + }) + + it("asks about command output when the command is still running after the ask delay", async () => { + vitest.useFakeTimers() + mockCline.ask.mockResolvedValue({ response: "messageResponse", text: "keep going", images: undefined }) + const terminal = await setupControllableTerminal() + + const handlePromise = handleCommand("sleep 60") + + await vitest.waitFor(() => expect(terminal.callbacks).toBeDefined()) + const callbacks = terminal.callbacks! + const proc = terminal.proc as unknown as RooTerminalProcess + + await callbacks.onLine("working...\n", proc) + + // First output alone must not trigger the ask. + expect(mockCline.ask).not.toHaveBeenCalled() + + await vitest.advanceTimersByTimeAsync(executeCommandModule.COMMAND_OUTPUT_ASK_DELAY_MS) + + expect(mockCline.ask).toHaveBeenCalledWith("command_output", "") + expect(terminal.proc.continue).toHaveBeenCalled() + + // Let the command finish so the tool can resolve. + await callbacks.onCompleted!("working...\n", proc) + callbacks.onShellExecutionComplete!({ exitCode: 0 }, proc) + terminal.resolveProcess() + await vitest.advanceTimersByTimeAsync(100) + + await handlePromise + + expect(mockPushToolResult).toHaveBeenCalled() + }) + }) }) From 0632cb7264c2ad5591940c7c1a71d9309d9ee0d4 Mon Sep 17 00:00:00 2001 From: Roomote Date: Wed, 29 Jul 2026 00:01:44 +0000 Subject: [PATCH 2/5] fix(terminal): anchor command_output ask delay to execution start Anchor the ask-delay timer to onShellExecutionStarted (falling back to the pre-runCommand timestamp) so shell-integration startup on cold terminals does not consume the grace period, and expand ask-policy tests to cover the agent-timeout cancel, re-anchor reschedule, ask error handling, and non-message responses to restore patch coverage. --- src/core/tools/ExecuteCommandTool.ts | 20 ++- .../__tests__/executeCommandTool.spec.ts | 160 +++++++++++++++++- 2 files changed, 176 insertions(+), 4 deletions(-) diff --git a/src/core/tools/ExecuteCommandTool.ts b/src/core/tools/ExecuteCommandTool.ts index b2858433f1..25e54f062c 100644 --- a/src/core/tools/ExecuteCommandTool.ts +++ b/src/core/tools/ExecuteCommandTool.ts @@ -353,7 +353,10 @@ export async function executeCommandInTerminal( // output and exit normally never prompt the user. The ask only fires if the // command is still running once COMMAND_OUTPUT_ASK_DELAY_MS has elapsed // since execution started, preserving the interrupt/feedback path for - // long-running commands. + // long-running commands. The anchor is re-based to onShellExecutionStarted + // (falling back to the pre-runCommand timestamp when that event never + // fires) so shell-integration startup on cold terminals does not consume + // the grace period. let commandStartedAt = 0 let commandOutputAskTimer: NodeJS.Timeout | undefined @@ -452,9 +455,21 @@ export async function executeCommandInTerminal( console.error("[ExecuteCommandTool] Failed to flush final command_output:", error) }) }, - onShellExecutionStarted: (pid: number | undefined) => { + onShellExecutionStarted: (pid: number | undefined, process: RooTerminalProcess) => { const status: CommandExecutionStatus = { executionId, status: "started", pid, command } provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) }) + + // Re-anchor the ask delay to actual execution start so the shell + // integration startup wait does not count against the grace period. + commandStartedAt = Date.now() + + // Output should not precede this event, but if it did, reschedule + // the pending ask against the corrected anchor. + if (commandOutputAskTimer) { + clearTimeout(commandOutputAskTimer) + commandOutputAskTimer = undefined + scheduleCommandOutputAsk(process) + } }, onShellExecutionComplete: (details: ExitCodeDetails) => { const status: CommandExecutionStatus = { executionId, status: "exited", exitCode: details.exitCode } @@ -481,6 +496,7 @@ export async function executeCommandInTerminal( workingDir = terminal.getCurrentWorkingDirectory() } + // Fallback anchor for providers that never fire onShellExecutionStarted. commandStartedAt = Date.now() const process = terminal.runCommand(command, callbacks) task.terminalProcess = process diff --git a/src/core/tools/__tests__/executeCommandTool.spec.ts b/src/core/tools/__tests__/executeCommandTool.spec.ts index ea412e0cd5..6c701acd79 100644 --- a/src/core/tools/__tests__/executeCommandTool.spec.ts +++ b/src/core/tools/__tests__/executeCommandTool.spec.ts @@ -368,9 +368,10 @@ describe("executeCommandTool", () => { return state } - const handleCommand = (command: string) => { + const handleCommand = (command: string, timeout?: number) => { mockToolUse.params.command = command - mockToolUse.nativeArgs = { command } + mockToolUse.params.timeout = timeout === undefined ? undefined : String(timeout) + mockToolUse.nativeArgs = timeout === undefined ? { command } : { command, timeout } return executeCommandTool.handle(mockCline as unknown as Task, mockToolUse, { askApproval: mockAskApproval as unknown as AskApproval, @@ -389,6 +390,7 @@ describe("executeCommandTool", () => { const callbacks = terminal.callbacks! const proc = terminal.proc as unknown as RooTerminalProcess + callbacks.onShellExecutionStarted!(1234, proc) await callbacks.onLine("hello\n", proc) await callbacks.onCompleted!("hello\n", proc) callbacks.onShellExecutionComplete!({ exitCode: 0 }, proc) @@ -418,6 +420,7 @@ describe("executeCommandTool", () => { const callbacks = terminal.callbacks! const proc = terminal.proc as unknown as RooTerminalProcess + callbacks.onShellExecutionStarted!(1234, proc) await callbacks.onLine("working...\n", proc) // First output alone must not trigger the ask. @@ -428,6 +431,10 @@ describe("executeCommandTool", () => { expect(mockCline.ask).toHaveBeenCalledWith("command_output", "") expect(terminal.proc.continue).toHaveBeenCalled() + // Further output after the ask must not schedule another ask. + await callbacks.onLine("still working...\n", proc) + expect(mockCline.ask).toHaveBeenCalledTimes(1) + // Let the command finish so the tool can resolve. await callbacks.onCompleted!("working...\n", proc) callbacks.onShellExecutionComplete!({ exitCode: 0 }, proc) @@ -438,5 +445,154 @@ describe("executeCommandTool", () => { expect(mockPushToolResult).toHaveBeenCalled() }) + + it("anchors the ask delay to execution start so shell integration startup does not consume it", async () => { + vitest.useFakeTimers() + const terminal = await setupControllableTerminal() + + const handlePromise = handleCommand("echo hello") + + await vitest.waitFor(() => expect(terminal.callbacks).toBeDefined()) + const callbacks = terminal.callbacks! + const proc = terminal.proc as unknown as RooTerminalProcess + + // Simulate a cold terminal spending most of the grace period waiting + // for shell integration before the command actually starts. + await vitest.advanceTimersByTimeAsync(executeCommandModule.COMMAND_OUTPUT_ASK_DELAY_MS - 2_000) + callbacks.onShellExecutionStarted!(1234, proc) + await callbacks.onLine("hello\n", proc) + + // Past the pre-runCommand anchor deadline but well within the window + // measured from execution start: still no ask. + await vitest.advanceTimersByTimeAsync(2_500) + expect(mockCline.ask).not.toHaveBeenCalled() + + await callbacks.onCompleted!("hello\n", proc) + callbacks.onShellExecutionComplete!({ exitCode: 0 }, proc) + terminal.resolveProcess() + await vitest.advanceTimersByTimeAsync(executeCommandModule.COMMAND_OUTPUT_ASK_DELAY_MS + 1_000) + + await handlePromise + + expect(mockCline.ask).not.toHaveBeenCalled() + expect(mockPushToolResult).toHaveBeenCalled() + }) + + it("re-anchors a pending ask when execution start is reported after early output", async () => { + vitest.useFakeTimers() + const terminal = await setupControllableTerminal() + + const handlePromise = handleCommand("echo hello") + + await vitest.waitFor(() => expect(terminal.callbacks).toBeDefined()) + const callbacks = terminal.callbacks! + const proc = terminal.proc as unknown as RooTerminalProcess + + // Output arrives before the execution-started event (defensive case). + await callbacks.onLine("hello\n", proc) + await vitest.advanceTimersByTimeAsync(executeCommandModule.COMMAND_OUTPUT_ASK_DELAY_MS - 2_000) + callbacks.onShellExecutionStarted!(1234, proc) + + // The pending ask was rescheduled against the new anchor, so the old + // deadline passing must not fire it. + await vitest.advanceTimersByTimeAsync(2_500) + expect(mockCline.ask).not.toHaveBeenCalled() + + await callbacks.onCompleted!("hello\n", proc) + callbacks.onShellExecutionComplete!({ exitCode: 0 }, proc) + terminal.resolveProcess() + await vitest.advanceTimersByTimeAsync(executeCommandModule.COMMAND_OUTPUT_ASK_DELAY_MS + 1_000) + + await handlePromise + + expect(mockCline.ask).not.toHaveBeenCalled() + }) + + it("cancels a pending ask when the agent timeout moves the command to the background", async () => { + vitest.useFakeTimers() + mockCline.supersedePendingAsk = vitest.fn() + const terminal = await setupControllableTerminal() + + const handlePromise = handleCommand("npm run dev", 2) + + await vitest.waitFor(() => expect(terminal.callbacks).toBeDefined()) + const callbacks = terminal.callbacks! + const proc = terminal.proc as unknown as RooTerminalProcess + + callbacks.onShellExecutionStarted!(1234, proc) + await callbacks.onLine("server starting...\n", proc) + + // Agent timeout (2s) fires before the ask delay (5s). + await vitest.advanceTimersByTimeAsync(2_000) + expect(terminal.proc.continue).toHaveBeenCalled() + expect(mockCline.supersedePendingAsk).toHaveBeenCalled() + + // Output after the background transition must never schedule an ask. + await callbacks.onLine("listening...\n", proc) + await vitest.advanceTimersByTimeAsync(executeCommandModule.COMMAND_OUTPUT_ASK_DELAY_MS + 1_000) + expect(mockCline.ask).not.toHaveBeenCalled() + + await handlePromise + + expect(mockPushToolResult).toHaveBeenCalled() + expect(mockPushToolResult.mock.calls[0][0]).toContain("still running") + }) + + it("swallows ask errors without failing the command", async () => { + vitest.useFakeTimers() + mockCline.ask.mockRejectedValue(new Error("Current ask promise was ignored")) + const terminal = await setupControllableTerminal() + + const handlePromise = handleCommand("sleep 60") + + await vitest.waitFor(() => expect(terminal.callbacks).toBeDefined()) + const callbacks = terminal.callbacks! + const proc = terminal.proc as unknown as RooTerminalProcess + + callbacks.onShellExecutionStarted!(1234, proc) + await callbacks.onLine("working...\n", proc) + await vitest.advanceTimersByTimeAsync(executeCommandModule.COMMAND_OUTPUT_ASK_DELAY_MS) + + expect(mockCline.ask).toHaveBeenCalledWith("command_output", "") + expect(terminal.proc.continue).not.toHaveBeenCalled() + + await callbacks.onCompleted!("working...\n", proc) + callbacks.onShellExecutionComplete!({ exitCode: 0 }, proc) + terminal.resolveProcess() + await vitest.advanceTimersByTimeAsync(100) + + await handlePromise + + expect(mockPushToolResult).toHaveBeenCalled() + expect(mockPushToolResult.mock.calls[0][0]).toContain("Exit code: 0") + }) + + it("does not continue the process when the ask is answered without a message", async () => { + vitest.useFakeTimers() + mockCline.ask.mockResolvedValue({ response: "yesButtonClicked", text: undefined, images: undefined }) + const terminal = await setupControllableTerminal() + + const handlePromise = handleCommand("sleep 60") + + await vitest.waitFor(() => expect(terminal.callbacks).toBeDefined()) + const callbacks = terminal.callbacks! + const proc = terminal.proc as unknown as RooTerminalProcess + + callbacks.onShellExecutionStarted!(1234, proc) + await callbacks.onLine("working...\n", proc) + await vitest.advanceTimersByTimeAsync(executeCommandModule.COMMAND_OUTPUT_ASK_DELAY_MS) + + expect(mockCline.ask).toHaveBeenCalledWith("command_output", "") + expect(terminal.proc.continue).not.toHaveBeenCalled() + + await callbacks.onCompleted!("working...\n", proc) + callbacks.onShellExecutionComplete!({ exitCode: 0 }, proc) + terminal.resolveProcess() + await vitest.advanceTimersByTimeAsync(100) + + await handlePromise + + expect(mockPushToolResult).toHaveBeenCalled() + }) }) }) From ac20ae69dd1254b593ff98e53bd62b83b03db86d Mon Sep 17 00:00:00 2001 From: Roomote Date: Wed, 29 Jul 2026 00:05:33 +0000 Subject: [PATCH 3/5] test(terminal): cover fallback anchor for command_output ask delay --- .../__tests__/executeCommandTool.spec.ts | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/core/tools/__tests__/executeCommandTool.spec.ts b/src/core/tools/__tests__/executeCommandTool.spec.ts index 6c701acd79..112db1a582 100644 --- a/src/core/tools/__tests__/executeCommandTool.spec.ts +++ b/src/core/tools/__tests__/executeCommandTool.spec.ts @@ -538,6 +538,33 @@ describe("executeCommandTool", () => { expect(mockPushToolResult.mock.calls[0][0]).toContain("still running") }) + it("falls back to the command dispatch time when execution start is never reported", async () => { + vitest.useFakeTimers() + mockCline.ask.mockResolvedValue({ response: "messageResponse", text: "keep going", images: undefined }) + const terminal = await setupControllableTerminal() + + const handlePromise = handleCommand("sleep 60") + + await vitest.waitFor(() => expect(terminal.callbacks).toBeDefined()) + const callbacks = terminal.callbacks! + const proc = terminal.proc as unknown as RooTerminalProcess + + // No onShellExecutionStarted: the pre-runCommand anchor applies. + await callbacks.onLine("working...\n", proc) + await vitest.advanceTimersByTimeAsync(executeCommandModule.COMMAND_OUTPUT_ASK_DELAY_MS) + + expect(mockCline.ask).toHaveBeenCalledWith("command_output", "") + + await callbacks.onCompleted!("working...\n", proc) + callbacks.onShellExecutionComplete!({ exitCode: 0 }, proc) + terminal.resolveProcess() + await vitest.advanceTimersByTimeAsync(100) + + await handlePromise + + expect(mockPushToolResult).toHaveBeenCalled() + }) + it("swallows ask errors without failing the command", async () => { vitest.useFakeTimers() mockCline.ask.mockRejectedValue(new Error("Current ask promise was ignored")) From ab70d8f6cd677da53067761ba8f352eb135968f2 Mon Sep 17 00:00:00 2001 From: Roomote Date: Wed, 29 Jul 2026 00:49:28 +0000 Subject: [PATCH 4/5] fix(terminal): clean up pending command_output ask on completion - Supersede a still-pending command_output ask in onCompleted so it resolves immediately instead of lingering until the next interactive message, and clear the Proceed/Kill controls in the webview when the final non-partial command_output say arrives. - Continue the process for any ask answer, not just typed messages, so a non-message response actually backgrounds the command and the tool resolves before the process completes. - Strengthen ask-policy tests: assert the rescheduled ask still fires at the re-anchored deadline, assert early resolution for non-message answers, and cover ask supersession on completion. --- src/core/tools/ExecuteCommandTool.ts | 13 +++- .../__tests__/executeCommandTool.spec.ts | 64 ++++++++++++++++--- webview-ui/src/components/chat/ChatView.tsx | 11 ++++ .../chat/__tests__/ChatView.spec.tsx | 63 ++++++++++++++++++ 4 files changed, 140 insertions(+), 11 deletions(-) diff --git a/src/core/tools/ExecuteCommandTool.ts b/src/core/tools/ExecuteCommandTool.ts index 25e54f062c..6e8a9becb4 100644 --- a/src/core/tools/ExecuteCommandTool.ts +++ b/src/core/tools/ExecuteCommandTool.ts @@ -374,8 +374,12 @@ export async function executeCommandInTerminal( if (response === "messageResponse") { message = { text, images } - process.continue() } + + // Any answer means the command should keep running in the background; + // continue the process so the tool resolves now instead of blocking + // until the command actually completes. + process.continue() } catch (_error) { // Silently handle ask errors (e.g., "Current ask promise was ignored") } @@ -422,6 +426,13 @@ export async function executeCommandInTerminal( clearTimeout(commandOutputAskTimer) commandOutputAskTimer = undefined + // If an interactive command_output ask is still pending, supersede it + // so it resolves immediately instead of lingering until the next + // interactive message bumps lastMessageTs. + if (hasAskedForCommandOutput && !runInBackground) { + task.supersedePendingAsk() + } + clearTimeout(pendingCommandOutputEmitTimer) pendingCommandOutputEmitTimer = undefined diff --git a/src/core/tools/__tests__/executeCommandTool.spec.ts b/src/core/tools/__tests__/executeCommandTool.spec.ts index 112db1a582..f077aadfb1 100644 --- a/src/core/tools/__tests__/executeCommandTool.spec.ts +++ b/src/core/tools/__tests__/executeCommandTool.spec.ts @@ -78,6 +78,7 @@ describe("executeCommandTool", () => { }, recordToolUsage: vitest.fn().mockReturnValue({} as ToolUsage), recordToolError: vitest.fn(), + supersedePendingAsk: vitest.fn(), providerRef: { deref: vitest.fn().mockResolvedValue({ getState: vitest.fn().mockResolvedValue({ @@ -357,7 +358,12 @@ describe("executeCommandTool", () => { const processPromise = new Promise((resolve) => { state.resolveProcess = resolve }) - state.proc = Object.assign(processPromise, { continue: vitest.fn(), abort: vitest.fn() }) + // Mirror real terminal behavior: continue() resolves the wait early + // while the command keeps running in the background. + state.proc = Object.assign(processPromise, { + continue: vitest.fn(() => state.resolveProcess()), + abort: vitest.fn(), + }) ;(TerminalRegistry.getOrCreateTerminal as ReturnType).mockResolvedValue({ runCommand: vitest.fn((_cmd: string, callbacks: RooTerminalCallbacks) => { state.callbacks = callbacks @@ -480,16 +486,17 @@ describe("executeCommandTool", () => { it("re-anchors a pending ask when execution start is reported after early output", async () => { vitest.useFakeTimers() + mockCline.ask.mockResolvedValue({ response: "messageResponse", text: "keep going", images: undefined }) const terminal = await setupControllableTerminal() - const handlePromise = handleCommand("echo hello") + const handlePromise = handleCommand("sleep 60") await vitest.waitFor(() => expect(terminal.callbacks).toBeDefined()) const callbacks = terminal.callbacks! const proc = terminal.proc as unknown as RooTerminalProcess // Output arrives before the execution-started event (defensive case). - await callbacks.onLine("hello\n", proc) + await callbacks.onLine("working...\n", proc) await vitest.advanceTimersByTimeAsync(executeCommandModule.COMMAND_OUTPUT_ASK_DELAY_MS - 2_000) callbacks.onShellExecutionStarted!(1234, proc) @@ -498,19 +505,23 @@ describe("executeCommandTool", () => { await vitest.advanceTimersByTimeAsync(2_500) expect(mockCline.ask).not.toHaveBeenCalled() - await callbacks.onCompleted!("hello\n", proc) + // The ask must still fire at the re-anchored deadline — a version + // that cleared the old timer without rescheduling would fail here. + await vitest.advanceTimersByTimeAsync(2_500) + expect(mockCline.ask).toHaveBeenCalledWith("command_output", "") + + // Let the command finish so the tool can resolve. + await callbacks.onCompleted!("working...\n", proc) callbacks.onShellExecutionComplete!({ exitCode: 0 }, proc) - terminal.resolveProcess() - await vitest.advanceTimersByTimeAsync(executeCommandModule.COMMAND_OUTPUT_ASK_DELAY_MS + 1_000) + await vitest.advanceTimersByTimeAsync(100) await handlePromise - expect(mockCline.ask).not.toHaveBeenCalled() + expect(mockPushToolResult).toHaveBeenCalled() }) it("cancels a pending ask when the agent timeout moves the command to the background", async () => { vitest.useFakeTimers() - mockCline.supersedePendingAsk = vitest.fn() const terminal = await setupControllableTerminal() const handlePromise = handleCommand("npm run dev", 2) @@ -594,7 +605,7 @@ describe("executeCommandTool", () => { expect(mockPushToolResult.mock.calls[0][0]).toContain("Exit code: 0") }) - it("does not continue the process when the ask is answered without a message", async () => { + it("resolves before completion when the ask is answered without a message", async () => { vitest.useFakeTimers() mockCline.ask.mockResolvedValue({ response: "yesButtonClicked", text: undefined, images: undefined }) const terminal = await setupControllableTerminal() @@ -609,9 +620,40 @@ describe("executeCommandTool", () => { await callbacks.onLine("working...\n", proc) await vitest.advanceTimersByTimeAsync(executeCommandModule.COMMAND_OUTPUT_ASK_DELAY_MS) + // Any ask answer backgrounds the command: the process is continued and + // the tool resolves without waiting for the command to complete. + // Note the process promise is never resolved in this test. + await vitest.advanceTimersByTimeAsync(100) + await handlePromise + expect(mockCline.ask).toHaveBeenCalledWith("command_output", "") - expect(terminal.proc.continue).not.toHaveBeenCalled() + expect(terminal.proc.continue).toHaveBeenCalled() + expect(mockPushToolResult).toHaveBeenCalled() + expect(mockPushToolResult.mock.calls[0][0]).toContain("still running") + + // Cleanup: let the command finish. + await callbacks.onCompleted!("working...\n", proc) + callbacks.onShellExecutionComplete!({ exitCode: 0 }, proc) + }) + + it("supersedes a pending ask when the command completes", async () => { + vitest.useFakeTimers() + mockCline.ask.mockReturnValue(new Promise(() => {})) + const terminal = await setupControllableTerminal() + + const handlePromise = handleCommand("sleep 5") + + await vitest.waitFor(() => expect(terminal.callbacks).toBeDefined()) + const callbacks = terminal.callbacks! + const proc = terminal.proc as unknown as RooTerminalProcess + callbacks.onShellExecutionStarted!(1234, proc) + await callbacks.onLine("working...\n", proc) + await vitest.advanceTimersByTimeAsync(executeCommandModule.COMMAND_OUTPUT_ASK_DELAY_MS) + + expect(mockCline.ask).toHaveBeenCalledWith("command_output", "") + + // The command completes while the ask is still pending. await callbacks.onCompleted!("working...\n", proc) callbacks.onShellExecutionComplete!({ exitCode: 0 }, proc) terminal.resolveProcess() @@ -619,7 +661,9 @@ describe("executeCommandTool", () => { await handlePromise + expect(mockCline.supersedePendingAsk).toHaveBeenCalled() expect(mockPushToolResult).toHaveBeenCalled() + expect(mockPushToolResult.mock.calls[0][0]).toContain("Exit code: 0") }) }) }) diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 01c4796c16..0410336609 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -462,6 +462,17 @@ const ChatViewComponent: React.ForwardRefRenderFunction { }), ) }) + + it("clears the command_output controls when the final output arrives", async () => { + const { getByTestId, getByRole, queryByRole } = renderChatView() + + // Hydrate state with a command_output ask (Proceed/Kill controls visible) + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: Date.now() - 2000, + text: "Initial task", + }, + { + type: "ask", + ask: "command_output", + ts: Date.now() - 1000, + text: "", + partial: false, + }, + ], + }) + + await waitFor(() => { + expect(getByTestId("chat-textarea")).toBeInTheDocument() + }) + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 50)) + }) + + expect(getByRole("button", { name: "chat:proceedWhileRunning.title" })).toBeInTheDocument() + + // The command completes: the final non-partial command_output say arrives. + mockPostMessage({ + clineMessages: [ + { + type: "say", + say: "task", + ts: Date.now() - 2000, + text: "Initial task", + }, + { + type: "ask", + ask: "command_output", + ts: Date.now() - 1000, + text: "", + partial: false, + }, + { + type: "say", + say: "command_output", + ts: Date.now(), + text: "done\n", + partial: false, + }, + ], + }) + + await waitFor(() => { + expect(queryByRole("button", { name: "chat:proceedWhileRunning.title" })).not.toBeInTheDocument() + }) + }) }) describe("ChatView - Follow-up Suggestions", () => { From 6027c4153cfc6394c41460c875eba0e82e6727e3 Mon Sep 17 00:00:00 2001 From: Roomote Date: Wed, 29 Jul 2026 00:55:22 +0000 Subject: [PATCH 5/5] test(terminal): note synthetic yesButtonClicked in non-message ask test --- src/core/tools/__tests__/executeCommandTool.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/core/tools/__tests__/executeCommandTool.spec.ts b/src/core/tools/__tests__/executeCommandTool.spec.ts index f077aadfb1..c3236f3565 100644 --- a/src/core/tools/__tests__/executeCommandTool.spec.ts +++ b/src/core/tools/__tests__/executeCommandTool.spec.ts @@ -606,6 +606,9 @@ describe("executeCommandTool", () => { }) it("resolves before completion when the ask is answered without a message", async () => { + // Note: in production only messageResponse answers reach a + // command_output ask (Proceed/Kill route through terminalOperation); + // yesButtonClicked is synthetic here to pin the non-message branch. vitest.useFakeTimers() mockCline.ask.mockResolvedValue({ response: "yesButtonClicked", text: undefined, images: undefined }) const terminal = await setupControllableTerminal()