Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions packages/types/src/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,18 @@ export const rooCodeEventsSchema = z.object({

[RooCodeEventName.TaskToolFailed]: z.tuple([z.string(), toolNamesSchema, z.string()]),
[RooCodeEventName.TaskTokenUsageUpdated]: z.tuple([z.string(), tokenUsageSchema]),

// Command Execution
taskCommandExecuted: z.tuple([
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new 'taskCommandExecuted' event is defined here but isn’t added to the RooCodeEventName enum nor included in the taskEventSchema discriminated union. Consistency between runtime validation and enum usage is crucial. Consider adding a corresponding enum entry (e.g. TaskCommandExecuted) and including it in the union.

z.string(),
z.object({
command: z.string(),
exitCode: z.number().optional(),
output: z.string(),
succeeded: z.boolean(),
failureReason: z.string().optional(),
}),
]),
})

export type RooCodeEvents = z.infer<typeof rooCodeEventsSchema>
Expand Down
12 changes: 12 additions & 0 deletions packages/types/src/task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,16 @@ export type TaskEvents = {
// Task Analytics
[RooCodeEventName.TaskToolFailed]: [taskId: string, tool: ToolName, error: string]
[RooCodeEventName.TaskTokenUsageUpdated]: [taskId: string, tokenUsage: TokenUsage]

// Command Execution
taskCommandExecuted: [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In TaskEvents, the 'taskCommandExecuted' event is added directly. For consistency and strong typing, consider referencing a centralized enum value (e.g. from RooCodeEventName) and ensuring that any runtime validation (like in taskEventSchema) includes this event.

Suggested change
taskCommandExecuted: [
\t[RooCodeEventName.TaskCommandExecuted]: [

taskId: string,
details: {
command: string
exitCode: number | undefined
output: string
succeeded: boolean
failureReason?: string
},
]
}
24 changes: 24 additions & 0 deletions src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,30 @@ import { AutoApprovalHandler } from "./AutoApprovalHandler"

const MAX_EXPONENTIAL_BACKOFF_SECONDS = 600 // 10 minutes

export type ClineEvents = {
message: [{ action: "created" | "updated"; message: ClineMessage }]
taskStarted: []
taskModeSwitched: [taskId: string, mode: string]
taskPaused: []
taskUnpaused: []
taskAskResponded: []
taskAborted: []
taskSpawned: [taskId: string]
taskCompleted: [taskId: string, tokenUsage: TokenUsage, toolUsage: ToolUsage]
taskTokenUsageUpdated: [taskId: string, tokenUsage: TokenUsage]
taskToolFailed: [taskId: string, tool: ToolName, error: string]
taskCommandExecuted: [
taskId: string,
details: {
command: string
exitCode: number | undefined
output: string
succeeded: boolean
failureReason?: string
},
]
}

export type TaskOptions = {
provider: ClineProvider
apiConfiguration: ProviderSettings
Expand Down
214 changes: 214 additions & 0 deletions src/core/tools/__tests__/executeCommand.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ describe("executeCommand", () => {
},
say: vitest.fn().mockResolvedValue(undefined),
terminalProcess: undefined,
emit: vitest.fn(),
}

// Create mock process that resolves immediately
Expand Down Expand Up @@ -471,4 +472,217 @@ describe("executeCommand", () => {
expect(mockTerminalInstance.getCurrentWorkingDirectory).toHaveBeenCalled()
})
})

describe("taskCommandExecuted Event", () => {
it("should emit taskCommandExecuted event when command completes successfully", async () => {
mockTerminal.getCurrentWorkingDirectory.mockReturnValue("/test/project")

// We need to mock Terminal.compressTerminalOutput since that's what sets the result
const mockCompressTerminalOutput = vitest.spyOn(Terminal, "compressTerminalOutput")
mockCompressTerminalOutput.mockReturnValue("Command output")

mockTerminal.runCommand.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => {
// Simulate async callback execution
setTimeout(() => {
callbacks.onShellExecutionStarted(1234, mockProcess)
callbacks.onCompleted("Command output", mockProcess)
callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess)
}, 0)
return mockProcess
})

const options: ExecuteCommandOptions = {
executionId: "test-123",
command: "echo test",
terminalShellIntegrationDisabled: false,
terminalOutputLineLimit: 500,
}

// Execute
const [rejected, result] = await executeCommand(mockTask, options)

// Verify
expect(rejected).toBe(false)
expect(mockTask.emit).toHaveBeenCalledWith("taskCommandExecuted", mockTask.taskId, {
command: "echo test",
exitCode: 0,
output: "Command output",
succeeded: true,
failureReason: undefined,
})

mockCompressTerminalOutput.mockRestore()
})

it("should emit taskCommandExecuted event when command fails with non-zero exit code", async () => {
mockTerminal.getCurrentWorkingDirectory.mockReturnValue("/test/project")

const mockCompressTerminalOutput = vitest.spyOn(Terminal, "compressTerminalOutput")
mockCompressTerminalOutput.mockReturnValue("Error output")

mockTerminal.runCommand.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => {
setTimeout(() => {
callbacks.onShellExecutionStarted(1234, mockProcess)
callbacks.onCompleted("Error output", mockProcess)
callbacks.onShellExecutionComplete({ exitCode: 1 }, mockProcess)
}, 0)
return mockProcess
})

const options: ExecuteCommandOptions = {
executionId: "test-123",
command: "exit 1",
terminalShellIntegrationDisabled: false,
terminalOutputLineLimit: 500,
}

// Execute
const [rejected, result] = await executeCommand(mockTask, options)

// Verify
expect(rejected).toBe(false)
expect(mockTask.emit).toHaveBeenCalledWith("taskCommandExecuted", mockTask.taskId, {
command: "exit 1",
exitCode: 1,
output: "Error output",
succeeded: false,
failureReason: expect.stringContaining("Command execution was not successful"),
})

mockCompressTerminalOutput.mockRestore()
})

it("should emit taskCommandExecuted event when command is terminated by signal", async () => {
mockTerminal.getCurrentWorkingDirectory.mockReturnValue("/test/project")

const mockCompressTerminalOutput = vitest.spyOn(Terminal, "compressTerminalOutput")
mockCompressTerminalOutput.mockReturnValue("Interrupted output")

mockTerminal.runCommand.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => {
setTimeout(() => {
callbacks.onShellExecutionStarted(1234, mockProcess)
callbacks.onCompleted("Interrupted output", mockProcess)
callbacks.onShellExecutionComplete(
{
exitCode: undefined,
signalName: "SIGTERM",
coreDumpPossible: false,
},
mockProcess,
)
}, 0)
return mockProcess
})

const options: ExecuteCommandOptions = {
executionId: "test-123",
command: "long-running-command",
terminalShellIntegrationDisabled: false,
terminalOutputLineLimit: 500,
}

// Execute
const [rejected, result] = await executeCommand(mockTask, options)

// Verify
expect(rejected).toBe(false)
expect(mockTask.emit).toHaveBeenCalledWith("taskCommandExecuted", mockTask.taskId, {
command: "long-running-command",
exitCode: undefined,
output: "Interrupted output",
succeeded: false,
failureReason: expect.stringContaining("Process terminated by signal SIGTERM"),
})

mockCompressTerminalOutput.mockRestore()
})

it("should emit taskCommandExecuted event when command times out", async () => {
// Mock the terminal process to not complete before timeout
let timeoutId: NodeJS.Timeout
const neverEndingProcess = new Promise<void>((resolve) => {
timeoutId = setTimeout(resolve, 10000) // Would resolve after 10 seconds
})
Object.assign(neverEndingProcess, {
continue: vitest.fn(),
abort: vitest.fn(() => {
clearTimeout(timeoutId)
}),
})

mockTerminal.runCommand.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => {
callbacks.onLine("Partial output", neverEndingProcess as any)
return neverEndingProcess
})

const options: ExecuteCommandOptions = {
executionId: "test-123",
command: "sleep 100",
terminalShellIntegrationDisabled: false,
terminalOutputLineLimit: 500,
commandExecutionTimeout: 100, // 100ms timeout
}

// Execute
const [rejected, result] = await executeCommand(mockTask, options)

// Verify
expect(rejected).toBe(false)
expect(result).toContain("terminated after exceeding")
expect(mockTask.emit).toHaveBeenCalledWith("taskCommandExecuted", mockTask.taskId, {
command: "sleep 100",
exitCode: undefined,
output: "Partial output",
succeeded: false,
failureReason: "Command timed out after 0.1s",
})
})

it("should emit taskCommandExecuted event when user provides feedback while command is running", async () => {
// Mock the ask function to simulate user feedback
mockTask.ask = vitest.fn().mockResolvedValue({
response: "messageResponse",
text: "Please stop the command",
images: [],
})

// Mock a long-running command
let commandResolve: () => void
const longRunningProcess = new Promise<void>((resolve) => {
commandResolve = resolve
})
Object.assign(longRunningProcess, {
continue: vitest.fn(() => {
// Simulate command continuing after feedback
setTimeout(() => commandResolve(), 10)
}),
})

mockTerminal.runCommand.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => {
// Simulate output that triggers user interaction
callbacks.onLine("Command is running...\n", longRunningProcess as any)
return longRunningProcess
})

const options: ExecuteCommandOptions = {
executionId: "test-123",
command: "npm install",
terminalShellIntegrationDisabled: false,
terminalOutputLineLimit: 500,
}

// Execute
const [rejected, result] = await executeCommand(mockTask, options)

// Verify
expect(rejected).toBe(true) // User feedback causes rejection
expect(mockTask.emit).toHaveBeenCalledWith("taskCommandExecuted", mockTask.taskId, {
command: "npm install",
exitCode: undefined,
output: "Command is running...\n",
succeeded: false,
failureReason: "Command is still running (user provided feedback)",
})
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,15 @@ describe("Command Execution Timeout Integration", () => {
// Mock task
mockTask = {
cwd: "/test/directory",
taskId: "test-task-123",
terminalProcess: undefined,
providerRef: {
deref: vitest.fn().mockResolvedValue({
postMessageToWebview: vitest.fn(),
}),
},
say: vitest.fn().mockResolvedValue(undefined),
emit: vitest.fn(),
}

// Mock terminal process
Expand Down Expand Up @@ -231,6 +233,7 @@ describe("Command Execution Timeout Integration", () => {
// Mock task with additional properties needed by executeCommandTool
mockTask = {
cwd: "/test/directory",
taskId: "test-task-123",
terminalProcess: undefined,
providerRef: {
deref: vitest.fn().mockResolvedValue({
Expand All @@ -251,6 +254,7 @@ describe("Command Execution Timeout Integration", () => {
lastMessageTs: Date.now(),
ask: vitest.fn(),
didRejectTool: false,
emit: vitest.fn(),
}
})

Expand Down
29 changes: 29 additions & 0 deletions src/core/tools/executeCommandTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,15 @@ export async function executeCommand(
await task.say("error", t("common:errors:command_timeout", { seconds: commandExecutionTimeoutSeconds }))
task.terminalProcess = undefined

// Emit taskCommandExecuted event for timeout
task.emit("taskCommandExecuted", task.taskId, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The task.emit('taskCommandExecuted', …) calls are repeated in several branches (e.g. timeout, user feedback, and completion). Consider refactoring these event emissions into a helper function to promote consistency and reduce duplication.

command,
exitCode: undefined,
output: accumulatedOutput, // Use accumulatedOutput instead of result
succeeded: false,
failureReason: `Command timed out after ${commandExecutionTimeoutSeconds}s`,
})

return [
false,
`The command was terminated after exceeding a user-configured ${commandExecutionTimeoutSeconds}s timeout. Do not try to re-run the command.`,
Expand Down Expand Up @@ -311,6 +320,15 @@ export async function executeCommand(
const { text, images } = message
await task.say("user_feedback", text, images)

// Emit taskCommandExecuted event for running command with user feedback
task.emit("taskCommandExecuted", task.taskId, {
command,
exitCode: undefined,
output: accumulatedOutput, // Use accumulatedOutput instead of result
succeeded: false,
failureReason: "Command is still running (user provided feedback)",
})

return [
true,
formatResponse.toolResult(
Expand All @@ -325,6 +343,7 @@ export async function executeCommand(
]
} else if (completed || exitDetails) {
let exitStatus: string = ""
let exitCode: number | undefined = exitDetails?.exitCode

if (exitDetails !== undefined) {
if (exitDetails.signalName) {
Expand All @@ -350,6 +369,16 @@ export async function executeCommand(

let workingDirInfo = ` within working directory '${terminal.getCurrentWorkingDirectory().toPosix()}'`

// Emit taskCommandExecuted event
const succeeded = exitCode === 0
task.emit("taskCommandExecuted", task.taskId, {
command,
exitCode,
output: result,
succeeded,
failureReason: succeeded ? undefined : exitStatus,
})

return [false, `Command executed in terminal ${workingDirInfo}. ${exitStatus}\nOutput:\n${result}`]
} else {
return [
Expand Down
Loading