fix: add user notifications for background task completion, errors, and timeouts

- Add BackgroundTaskRunnerCallbacks interface with onTaskTimeout and onTaskError
- Wire VS Code notifications in ClineProvider: info on completion, warning on timeout/error
- Document auto-approval design decision for read-only background tasks
- Add 2 new tests for callback invocation (19 total, all passing)
This commit is contained in:
Roo Code 2026-05-12 10:05:30 +00:00
parent 437c9e8e63
commit a599babda1
4 changed files with 78 additions and 6 deletions

View file

@ -35,17 +35,31 @@ export interface BackgroundTaskInfo {
timeoutHandle: ReturnType<typeof setTimeout>
}
/**
* Optional callbacks that allow the owner (e.g. ClineProvider) to react to
* background task lifecycle events such as completion, timeout, or errors.
*/
export interface BackgroundTaskRunnerCallbacks {
/** Called when a background task times out. */
onTaskTimeout?: (taskId: string, parentTaskId: string) => void
/** Called when aborting a background task throws an error. */
onTaskError?: (taskId: string, parentTaskId: string, error: Error) => void
}
export class BackgroundTaskRunner {
private backgroundTasks: Map<string, BackgroundTaskInfo> = new Map()
private maxConcurrentTasks: number
private taskTimeoutMs: number
private callbacks: BackgroundTaskRunnerCallbacks
constructor(
maxConcurrentTasks: number = DEFAULT_MAX_BACKGROUND_TASKS,
taskTimeoutMs: number = DEFAULT_BACKGROUND_TASK_TIMEOUT_MS,
callbacks: BackgroundTaskRunnerCallbacks = {},
) {
this.maxConcurrentTasks = maxConcurrentTasks
this.taskTimeoutMs = taskTimeoutMs
this.callbacks = callbacks
}
/**
@ -163,11 +177,13 @@ export class BackgroundTaskRunner {
try {
await info.task.abortTask(true)
} catch (error) {
console.error(
`[BackgroundTaskRunner] Error aborting background task ${taskId}: ${
error instanceof Error ? error.message : String(error)
}`,
)
const err = error instanceof Error ? error : new Error(String(error))
console.error(`[BackgroundTaskRunner] Error aborting background task ${taskId}: ${err.message}`)
try {
this.callbacks.onTaskError?.(taskId, info.parentTaskId, err)
} catch {
// Callback errors must not break cleanup.
}
}
this.backgroundTasks.delete(taskId)
@ -193,7 +209,17 @@ export class BackgroundTaskRunner {
* Handle timeout of a background task.
*/
private async timeoutTask(taskId: string): Promise<void> {
const info = this.backgroundTasks.get(taskId)
const parentTaskId = info?.parentTaskId ?? "unknown"
console.warn(`[BackgroundTaskRunner] Background task ${taskId} timed out after ${this.taskTimeoutMs}ms`)
try {
this.callbacks.onTaskTimeout?.(taskId, parentTaskId)
} catch {
// Callback errors must not break cleanup.
}
await this.cancelTask(taskId)
}
}

View file

@ -1397,6 +1397,12 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
let timeouts: NodeJS.Timeout[] = []
// Background tasks auto-approve all asks immediately (no user interaction).
// Design decision: Full auto-approval is safe here because background tasks
// are restricted to read-only tools only (read_file, list_files, search_files,
// codebase_search). They cannot modify files, execute commands, or perform any
// destructive operations. If a future phase introduces write-capable background
// tasks, this auto-approval should be revisited to allow selective user input
// for dangerous operations.
if (this.isBackgroundTask) {
this.approveAsk()
await pWaitFor(() => this.askResponse !== undefined || this.lastMessageTs !== askTs, { interval: 100 })

View file

@ -129,6 +129,19 @@ describe("BackgroundTaskRunner", () => {
it("should handle canceling unknown task gracefully", async () => {
await runner.cancelTask("unknown") // should not throw
})
it("should invoke onTaskError callback when abort throws", async () => {
const onTaskError = vi.fn()
const customRunner = new BackgroundTaskRunner(3, undefined, { onTaskError })
const task = createMockTask("task-1")
task.abortTask.mockRejectedValue(new Error("abort failed"))
customRunner.registerTask(task, "parent-1")
await customRunner.cancelTask("task-1")
expect(onTaskError).toHaveBeenCalledWith("task-1", "parent-1", expect.any(Error))
expect(customRunner.activeCount).toBe(0)
})
})
describe("cancelTasksByParent", () => {
@ -164,6 +177,18 @@ describe("BackgroundTaskRunner", () => {
expect(task.abortTask).toHaveBeenCalledWith(true)
expect(customRunner.activeCount).toBe(0)
})
it("should invoke onTaskTimeout callback when task times out", async () => {
const onTaskTimeout = vi.fn()
const customRunner = new BackgroundTaskRunner(3, 5000, { onTaskTimeout })
const task = createMockTask("task-1")
customRunner.registerTask(task, "parent-1")
vi.advanceTimersByTime(5000)
await vi.runAllTimersAsync()
expect(onTaskTimeout).toHaveBeenCalledWith("task-1", "parent-1")
})
})
describe("dispose", () => {

View file

@ -85,6 +85,11 @@ import { CustomModesManager } from "../config/CustomModesManager"
import { Task } from "../task/Task"
import { buildTaskContext } from "../task/TaskContextBuilder"
import { BackgroundTaskRunner, BACKGROUND_TASK_ALLOWED_TOOLS } from "../task/BackgroundTaskRunner"
import {
BackgroundTaskRunner,
BACKGROUND_TASK_ALLOWED_TOOLS,
BackgroundTaskRunnerCallbacks,
} from "../task/BackgroundTaskRunner"
import { webviewMessageHandler } from "./webviewMessageHandler"
import type { ClineMessage, TodoItem, SubtaskQueueItem, TaskPermissions, ContextHandoffSummary } from "@roo-code/types"
@ -144,7 +149,14 @@ export class ClineProvider
private recentTasksCache?: string[]
public readonly taskHistoryStore: TaskHistoryStore
private taskHistoryStoreInitialized = false
public readonly backgroundTaskRunner: BackgroundTaskRunner = new BackgroundTaskRunner()
public readonly backgroundTaskRunner: BackgroundTaskRunner = new BackgroundTaskRunner(undefined, undefined, {
onTaskTimeout: (taskId, _parentTaskId) => {
vscode.window.showWarningMessage(`Background task ${taskId} timed out and was cancelled.`)
},
onTaskError: (taskId, _parentTaskId, error) => {
vscode.window.showWarningMessage(`Background task ${taskId} encountered an error: ${error.message}`)
},
})
private globalStateWriteThroughTimer: ReturnType<typeof setTimeout> | null = null
private static readonly GLOBAL_STATE_WRITE_THROUGH_DEBOUNCE_MS = 5000 // 5 seconds
private pendingOperations: Map<string, PendingEditOperation> = new Map()
@ -3265,6 +3277,9 @@ export class ClineProvider
return
}
// Notify the user that the background task finished.
vscode.window.showInformationMessage(`Background task ${taskId} completed.`)
const parentTaskId = info.parentTaskId
const currentTask = this.getCurrentTask()