mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-10-09 03:17:58 +00:00
Merge 636254ac5a into 2bb826039b
This commit is contained in:
commit
c875d1e948
3 changed files with 199 additions and 1 deletions
|
|
@ -3,6 +3,7 @@ import * as path from "path"
|
||||||
import * as vscode from "vscode"
|
import * as vscode from "vscode"
|
||||||
|
|
||||||
import delay from "delay"
|
import delay from "delay"
|
||||||
|
import { parse } from "shell-quote"
|
||||||
|
|
||||||
import { CommandExecutionStatus, DEFAULT_TERMINAL_OUTPUT_PREVIEW_SIZE, PersistedCommandOutput } from "@roo-code/types"
|
import { CommandExecutionStatus, DEFAULT_TERMINAL_OUTPUT_PREVIEW_SIZE, PersistedCommandOutput } from "@roo-code/types"
|
||||||
import { TelemetryService } from "@roo-code/telemetry"
|
import { TelemetryService } from "@roo-code/telemetry"
|
||||||
|
|
@ -29,6 +30,53 @@ interface ExecuteCommandParams {
|
||||||
timeout?: number | null
|
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 {
|
export function resolveAgentTimeoutMs(timeoutSeconds: number | null | undefined): number {
|
||||||
const requestedAgentTimeout = typeof timeoutSeconds === "number" && timeoutSeconds > 0 ? timeoutSeconds * 1000 : 0
|
const requestedAgentTimeout = typeof timeoutSeconds === "number" && timeoutSeconds > 0 ? timeoutSeconds * 1000 : 0
|
||||||
|
|
||||||
|
|
@ -192,6 +240,7 @@ export async function executeCommandInTerminal(
|
||||||
|
|
||||||
let message: { text?: string; images?: string[] } | undefined
|
let message: { text?: string; images?: string[] } | undefined
|
||||||
let runInBackground = false
|
let runInBackground = false
|
||||||
|
const requestedBackgroundExecution = hasTrailingBackgroundOperator(command)
|
||||||
let completed = false
|
let completed = false
|
||||||
let result: string = ""
|
let result: string = ""
|
||||||
let persistedResult: PersistedCommandOutput | undefined
|
let persistedResult: PersistedCommandOutput | undefined
|
||||||
|
|
@ -380,6 +429,11 @@ export async function executeCommandInTerminal(
|
||||||
const process = terminal.runCommand(command, callbacks)
|
const process = terminal.runCommand(command, callbacks)
|
||||||
task.terminalProcess = process
|
task.terminalProcess = process
|
||||||
|
|
||||||
|
if (requestedBackgroundExecution) {
|
||||||
|
runInBackground = true
|
||||||
|
process.continue()
|
||||||
|
}
|
||||||
|
|
||||||
// Dual-timeout logic:
|
// Dual-timeout logic:
|
||||||
// - Agent timeout: transitions the command to background (continues running)
|
// - Agent timeout: transitions the command to background (continues running)
|
||||||
// - User timeout: aborts the command (kills it)
|
// - 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")
|
vitest.mock("../../../integrations/terminal/ExecaTerminal")
|
||||||
|
|
||||||
// Import the actual executeCommand function (not mocked)
|
// Import the actual executeCommand function (not mocked)
|
||||||
import { executeCommandInTerminal } from "../ExecuteCommandTool"
|
import { executeCommandInTerminal, hasTrailingBackgroundOperator } from "../ExecuteCommandTool"
|
||||||
|
|
||||||
// Tests for the executeCommand function
|
// Tests for the executeCommand function
|
||||||
describe("executeCommand", () => {
|
describe("executeCommand", () => {
|
||||||
|
|
@ -75,6 +75,28 @@ describe("executeCommand", () => {
|
||||||
;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(mockTerminal)
|
;(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", () => {
|
describe("Working Directory Behavior", () => {
|
||||||
it("should use terminal.getCurrentWorkingDirectory() in the output message for completed commands", async () => {
|
it("should use terminal.getCurrentWorkingDirectory() in the output message for completed commands", async () => {
|
||||||
// Setup: Mock terminal to return a different current working directory
|
// Setup: Mock terminal to return a different current working directory
|
||||||
|
|
@ -311,6 +333,30 @@ describe("executeCommand", () => {
|
||||||
})
|
})
|
||||||
|
|
||||||
describe("Command Execution States", () => {
|
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 () => {
|
it("should handle completed command with exit code 0", async () => {
|
||||||
mockTerminal.getCurrentWorkingDirectory.mockReturnValue("/test/project")
|
mockTerminal.getCurrentWorkingDirectory.mockReturnValue("/test/project")
|
||||||
mockTerminal.runCommand.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => {
|
mockTerminal.runCommand.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => {
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue