mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
Merge 636254ac5a into b867ec9145
This commit is contained in:
commit
34583f8dbd
3 changed files with 199 additions and 1 deletions
|
|
@ -3,6 +3,7 @@ import * as path from "path"
|
|||
import * as vscode from "vscode"
|
||||
|
||||
import delay from "delay"
|
||||
import { parse } from "shell-quote"
|
||||
|
||||
import { CommandExecutionStatus, DEFAULT_TERMINAL_OUTPUT_PREVIEW_SIZE, PersistedCommandOutput } from "@roo-code/types"
|
||||
|
||||
|
|
@ -28,6 +29,53 @@ interface ExecuteCommandParams {
|
|||
timeout?: number | null
|
||||
}
|
||||
|
||||
function isTrailingBackgroundOperatorToken(token: unknown): boolean {
|
||||
if (typeof token !== "object" || token === null) {
|
||||
return false
|
||||
}
|
||||
const entry = token as Record<string, unknown>
|
||||
return entry.op === "&"
|
||||
}
|
||||
|
||||
function isSkippableTrailingToken(token: unknown): boolean {
|
||||
if (typeof token === "string") {
|
||||
return token.trim().length === 0
|
||||
}
|
||||
if (typeof token !== "object" || token === null) {
|
||||
return false
|
||||
}
|
||||
const entry = token as Record<string, unknown>
|
||||
return typeof entry.comment === "string"
|
||||
}
|
||||
|
||||
/**
|
||||
* Detect explicit shell backgrounding intent, e.g. `pnpm dev &`.
|
||||
*
|
||||
* We only treat a trailing standalone `&` as background intent so commands like
|
||||
* `foo && bar` or `sleep 1 & echo done` remain foreground in agent execution flow.
|
||||
*/
|
||||
export function hasTrailingBackgroundOperator(command: string): boolean {
|
||||
const trimmed = command.trim()
|
||||
if (!trimmed) {
|
||||
return false
|
||||
}
|
||||
|
||||
try {
|
||||
const tokens = parse(trimmed)
|
||||
for (let i = tokens.length - 1; i >= 0; i--) {
|
||||
const token = tokens[i]
|
||||
if (isSkippableTrailingToken(token)) {
|
||||
continue
|
||||
}
|
||||
return isTrailingBackgroundOperatorToken(token)
|
||||
}
|
||||
return false
|
||||
} catch {
|
||||
// If parsing fails, default to foreground behavior.
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
export function resolveAgentTimeoutMs(timeoutSeconds: number | null | undefined): number {
|
||||
const requestedAgentTimeout = typeof timeoutSeconds === "number" && timeoutSeconds > 0 ? timeoutSeconds * 1000 : 0
|
||||
|
||||
|
|
@ -191,6 +239,7 @@ export async function executeCommandInTerminal(
|
|||
|
||||
let message: { text?: string; images?: string[] } | undefined
|
||||
let runInBackground = false
|
||||
const requestedBackgroundExecution = hasTrailingBackgroundOperator(command)
|
||||
let completed = false
|
||||
let result: string = ""
|
||||
let persistedResult: PersistedCommandOutput | undefined
|
||||
|
|
@ -372,6 +421,11 @@ export async function executeCommandInTerminal(
|
|||
const process = terminal.runCommand(command, callbacks)
|
||||
task.terminalProcess = process
|
||||
|
||||
if (requestedBackgroundExecution) {
|
||||
runInBackground = true
|
||||
process.continue()
|
||||
}
|
||||
|
||||
// Dual-timeout logic:
|
||||
// - Agent timeout: transitions the command to background (continues running)
|
||||
// - User timeout: aborts the command (kills it)
|
||||
|
|
|
|||
98
src/core/tools/__tests__/executeCommand.integration.spec.ts
Normal file
98
src/core/tools/__tests__/executeCommand.integration.spec.ts
Normal file
|
|
@ -0,0 +1,98 @@
|
|||
import { randomUUID } from "crypto"
|
||||
import * as os from "os"
|
||||
import * as path from "path"
|
||||
import * as fs from "fs/promises"
|
||||
|
||||
import { Task } from "../../task/Task"
|
||||
|
||||
import { executeCommandInTerminal } from "../ExecuteCommandTool"
|
||||
|
||||
describe("executeCommandInTerminal integration", () => {
|
||||
it("returns promptly for explicit trailing background commands", async () => {
|
||||
if (process.platform === "win32") {
|
||||
return
|
||||
}
|
||||
|
||||
const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "roo-execute-command-bg-"))
|
||||
const taskId = `test-task-${randomUUID()}`
|
||||
|
||||
try {
|
||||
const provider = {
|
||||
postMessageToWebview: vitest.fn(),
|
||||
getState: vitest.fn().mockResolvedValue({}),
|
||||
context: undefined,
|
||||
}
|
||||
|
||||
const task = {
|
||||
cwd,
|
||||
taskId,
|
||||
providerRef: {
|
||||
deref: vitest.fn().mockResolvedValue(provider),
|
||||
},
|
||||
say: vitest.fn().mockResolvedValue(undefined),
|
||||
ask: vitest.fn(),
|
||||
terminalProcess: undefined,
|
||||
supersedePendingAsk: vitest.fn(),
|
||||
} as unknown as Task
|
||||
|
||||
const startedAt = Date.now()
|
||||
const [rejected, result] = await executeCommandInTerminal(task, {
|
||||
executionId: "integration-bg-1",
|
||||
command: "sleep 2 &",
|
||||
terminalShellIntegrationDisabled: true,
|
||||
})
|
||||
const elapsedMs = Date.now() - startedAt
|
||||
|
||||
expect(rejected).toBe(false)
|
||||
expect(elapsedMs).toBeLessThan(1_200)
|
||||
expect(result).toContain("Command is still running in terminal")
|
||||
} finally {
|
||||
await fs.rm(cwd, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
it("does not treat mid-command backgrounding as explicit background execution", async () => {
|
||||
if (process.platform === "win32") {
|
||||
return
|
||||
}
|
||||
|
||||
const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "roo-execute-command-mid-bg-"))
|
||||
const taskId = `test-task-${randomUUID()}`
|
||||
|
||||
try {
|
||||
const provider = {
|
||||
postMessageToWebview: vitest.fn(),
|
||||
getState: vitest.fn().mockResolvedValue({}),
|
||||
context: undefined,
|
||||
}
|
||||
|
||||
const task = {
|
||||
cwd,
|
||||
taskId,
|
||||
providerRef: {
|
||||
deref: vitest.fn().mockResolvedValue(provider),
|
||||
},
|
||||
say: vitest.fn().mockResolvedValue(undefined),
|
||||
ask: vitest.fn(),
|
||||
terminalProcess: undefined,
|
||||
supersedePendingAsk: vitest.fn(),
|
||||
} as unknown as Task
|
||||
|
||||
const startedAt = Date.now()
|
||||
const [rejected, result] = await executeCommandInTerminal(task, {
|
||||
executionId: "integration-bg-2",
|
||||
command: "sleep 1 & sleep 1; echo done",
|
||||
terminalShellIntegrationDisabled: true,
|
||||
})
|
||||
const elapsedMs = Date.now() - startedAt
|
||||
|
||||
expect(rejected).toBe(false)
|
||||
expect(elapsedMs).toBeGreaterThan(700)
|
||||
expect(result).toContain("Exit code: 0")
|
||||
expect(result).toContain("done")
|
||||
expect(result).not.toContain("Command is still running in terminal")
|
||||
} finally {
|
||||
await fs.rm(cwd, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
})
|
||||
|
|
@ -21,7 +21,7 @@ vitest.mock("../../../integrations/terminal/Terminal")
|
|||
vitest.mock("../../../integrations/terminal/ExecaTerminal")
|
||||
|
||||
// Import the actual executeCommand function (not mocked)
|
||||
import { executeCommandInTerminal } from "../ExecuteCommandTool"
|
||||
import { executeCommandInTerminal, hasTrailingBackgroundOperator } from "../ExecuteCommandTool"
|
||||
|
||||
// Tests for the executeCommand function
|
||||
describe("executeCommand", () => {
|
||||
|
|
@ -75,6 +75,28 @@ describe("executeCommand", () => {
|
|||
;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(mockTerminal)
|
||||
})
|
||||
|
||||
describe("Background Command Detection", () => {
|
||||
it("detects a trailing standalone background operator", () => {
|
||||
expect(hasTrailingBackgroundOperator("pnpm dev &")).toBe(true)
|
||||
})
|
||||
|
||||
it("does not treat && as background execution", () => {
|
||||
expect(hasTrailingBackgroundOperator("pnpm lint && pnpm test")).toBe(false)
|
||||
})
|
||||
|
||||
it("does not treat quoted ampersands as background execution", () => {
|
||||
expect(hasTrailingBackgroundOperator('echo "a & b"')).toBe(false)
|
||||
})
|
||||
|
||||
it("does not treat mid-command backgrounding as trailing background execution", () => {
|
||||
expect(hasTrailingBackgroundOperator("sleep 1 & echo done")).toBe(false)
|
||||
})
|
||||
|
||||
it("ignores parse failures and defaults to foreground", () => {
|
||||
expect(hasTrailingBackgroundOperator("echo 'unterminated")).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe("Working Directory Behavior", () => {
|
||||
it("should use terminal.getCurrentWorkingDirectory() in the output message for completed commands", async () => {
|
||||
// Setup: Mock terminal to return a different current working directory
|
||||
|
|
@ -311,6 +333,30 @@ describe("executeCommand", () => {
|
|||
})
|
||||
|
||||
describe("Command Execution States", () => {
|
||||
it("continues immediately for explicit trailing background commands", async () => {
|
||||
let resolveProcess: (() => void) | undefined
|
||||
const continueSpy = vitest.fn(() => resolveProcess?.())
|
||||
const backgroundProcess = new Promise<void>((resolve) => {
|
||||
resolveProcess = resolve
|
||||
}) as Promise<void> & { continue: () => void }
|
||||
|
||||
backgroundProcess.continue = continueSpy
|
||||
|
||||
mockTerminal.runCommand.mockReturnValue(backgroundProcess)
|
||||
|
||||
const options: ExecuteCommandOptions = {
|
||||
executionId: "test-123",
|
||||
command: "pnpm dev &",
|
||||
terminalShellIntegrationDisabled: true,
|
||||
}
|
||||
|
||||
const [rejected, result] = await executeCommandInTerminal(mockTask, options)
|
||||
|
||||
expect(rejected).toBe(false)
|
||||
expect(continueSpy).toHaveBeenCalledTimes(1)
|
||||
expect(result).toContain("Command is still running in terminal")
|
||||
})
|
||||
|
||||
it("should handle completed command with exit code 0", async () => {
|
||||
mockTerminal.getCurrentWorkingDirectory.mockReturnValue("/test/project")
|
||||
mockTerminal.runCommand.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue