From 4b9c7ac155eaa80abb3ef137367a7a2058134212 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Tue, 12 May 2026 03:28:52 +0000 Subject: [PATCH 01/11] feat: implement sequential fan-out / fan-in for orchestrator (Phase 2 of #12330) Adds subtask queue support to the new_task tool, allowing the orchestrator to define multiple subtasks that execute automatically in sequence without returning to the parent between each one. This saves LLM API calls and enables more efficient multi-agent workflows. Key changes: - SubtaskQueueItem, SubtaskResult types in packages/types/src/history.ts - task_queue parameter on new_task tool (optional JSON array) - NewTaskTool parses and validates queued subtasks, stores on parent - delegateParentAndOpenChild persists queue in parent HistoryItem - reopenParentFromDelegation auto-advances queue via advanceSubtaskQueue - formatAggregatedQueueResults aggregates all results when queue completes - 9 new tests covering queue advance, exhaustion, and result formatting - All 56 existing delegation tests continue to pass --- packages/types/src/history.ts | 26 ++ src/__tests__/sequential-fan-out.spec.ts | 242 ++++++++++++++++++ .../assistant-message/NativeToolCallParser.ts | 2 + .../prompts/tools/native-tools/new_task.ts | 10 +- src/core/tools/NewTaskTool.ts | 49 +++- src/core/webview/ClineProvider.ts | 175 ++++++++++++- src/shared/tools.ts | 5 +- 7 files changed, 498 insertions(+), 11 deletions(-) create mode 100644 src/__tests__/sequential-fan-out.spec.ts diff --git a/packages/types/src/history.ts b/packages/types/src/history.ts index 302fbe3454c..2b0a681f48d 100644 --- a/packages/types/src/history.ts +++ b/packages/types/src/history.ts @@ -1,5 +1,27 @@ import { z } from "zod" +/** + * SubtaskQueueItem — a single queued subtask definition for sequential fan-out. + * Used by the orchestrator to define a pipeline of subtasks that execute one after another. + */ +export const subtaskQueueItemSchema = z.object({ + mode: z.string(), + message: z.string(), +}) + +export type SubtaskQueueItem = z.infer + +/** + * SubtaskResult — the result of a completed subtask in a queue. + */ +export const subtaskResultSchema = z.object({ + taskId: z.string(), + mode: z.string(), + summary: z.string(), +}) + +export type SubtaskResult = z.infer + /** * HistoryItem */ @@ -26,6 +48,10 @@ export const historyItemSchema = z.object({ awaitingChildId: z.string().optional(), // Child currently awaited (set when delegated) completedByChildId: z.string().optional(), // Child that completed and resumed this parent completionResultSummary: z.string().optional(), // Summary from completed child + // Sequential fan-out queue (Phase 2) + subtaskQueue: z.array(subtaskQueueItemSchema).optional(), // Remaining subtasks to execute + subtaskQueueIndex: z.number().optional(), // Current position in the original queue (0-based) + subtaskResults: z.array(subtaskResultSchema).optional(), // Results from completed queue subtasks }) export type HistoryItem = z.infer diff --git a/src/__tests__/sequential-fan-out.spec.ts b/src/__tests__/sequential-fan-out.spec.ts new file mode 100644 index 00000000000..44d7a4d41e4 --- /dev/null +++ b/src/__tests__/sequential-fan-out.spec.ts @@ -0,0 +1,242 @@ +/** + * Tests for Phase 2: Sequential fan-out / fan-in. + * + * Tests the subtask queue mechanism where an orchestrator can define + * multiple subtasks that execute one after another with automatic transitions. + */ + +import { describe, it, expect, vi } from "vitest" +import { RooCodeEventName } from "@roo-code/types" +import type { HistoryItem, SubtaskQueueItem } from "@roo-code/types" + +import { ClineProvider } from "../core/webview/ClineProvider" + +describe("Sequential fan-out queue types", () => { + it("SubtaskQueueItem has required mode and message fields", () => { + const item: SubtaskQueueItem = { mode: "code", message: "Implement feature X" } + expect(item.mode).toBe("code") + expect(item.message).toBe("Implement feature X") + }) + + it("HistoryItem can include subtask queue fields", () => { + const historyItem: Partial = { + id: "test-1", + subtaskQueue: [ + { mode: "code", message: "Step 1" }, + { mode: "debug", message: "Step 2" }, + ], + subtaskQueueIndex: 0, + subtaskResults: [{ taskId: "child-1", mode: "code", summary: "Done" }], + } + expect(historyItem.subtaskQueue).toHaveLength(2) + expect(historyItem.subtaskQueueIndex).toBe(0) + expect(historyItem.subtaskResults).toHaveLength(1) + }) + + it("HistoryItem subtask queue fields are optional", () => { + const historyItem: Partial = { + id: "test-2", + status: "active", + } + expect(historyItem.subtaskQueue).toBeUndefined() + expect(historyItem.subtaskQueueIndex).toBeUndefined() + expect(historyItem.subtaskResults).toBeUndefined() + }) +}) + +describe("advanceSubtaskQueue", () => { + const makeHistoryItem = (overrides: Partial = {}): HistoryItem => ({ + id: "parent-1", + number: 1, + ts: Date.now(), + task: "Test task", + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + status: "delegated", + ...overrides, + }) + + it("returns handled=true when there are more subtasks in the queue", async () => { + const emitSpy = vi.fn() + const mockChild = { taskId: "child-2", start: vi.fn() } + const provider = { + getCurrentTask: vi.fn().mockReturnValue({ taskId: "child-1" }), + removeClineFromStack: vi.fn().mockResolvedValue(undefined), + getTaskWithId: vi.fn().mockResolvedValue({ + historyItem: makeHistoryItem({ id: "child-1", status: "active" }), + }), + updateTaskHistory: vi.fn().mockResolvedValue(undefined), + handleModeSwitch: vi.fn().mockResolvedValue(undefined), + createTask: vi.fn().mockResolvedValue(mockChild), + emit: emitSpy, + log: vi.fn(), + } + + const subtaskQueue: SubtaskQueueItem[] = [ + { mode: "code", message: "Step 1" }, + { mode: "debug", message: "Step 2" }, + ] + + const historyItem = makeHistoryItem({ + subtaskQueue, + subtaskQueueIndex: 0, + subtaskResults: [], + childIds: ["child-1"], + }) + + const result = await (ClineProvider.prototype as any).advanceSubtaskQueue.call(provider, { + parentTaskId: "parent-1", + childTaskId: "child-1", + completionResultSummary: "Step 1 done", + historyItem, + }) + + expect(result.handled).toBe(true) + + // Should have closed the current child + expect(provider.removeClineFromStack).toHaveBeenCalled() + + // Should have marked child as completed + expect(provider.updateTaskHistory).toHaveBeenCalledWith( + expect.objectContaining({ id: "child-1", status: "completed" }), + ) + + // Should have switched mode to next subtask's mode + expect(provider.handleModeSwitch).toHaveBeenCalledWith("debug") + + // Should have created the next child with the queued message + expect(provider.createTask).toHaveBeenCalledWith("Step 2", undefined, undefined, { + initialTodos: [], + initialStatus: "active", + startTask: false, + }) + + // Should have started the next child + expect(mockChild.start).toHaveBeenCalled() + + // Should have updated parent with advanced queue index + expect(provider.updateTaskHistory).toHaveBeenCalledWith( + expect.objectContaining({ + id: "parent-1", + subtaskQueueIndex: 1, + subtaskResults: [{ taskId: "child-1", mode: "unknown", summary: "Step 1 done" }], + awaitingChildId: "child-2", + delegatedToId: "child-2", + }), + ) + + // Should have emitted delegation events + expect(emitSpy).toHaveBeenCalledWith( + RooCodeEventName.TaskDelegationCompleted, + "parent-1", + "child-1", + "Step 1 done", + ) + expect(emitSpy).toHaveBeenCalledWith(RooCodeEventName.TaskDelegated, "parent-1", "child-2") + }) + + it("returns handled=false with aggregated summary when queue is exhausted", async () => { + const provider = { + getCurrentTask: vi.fn().mockReturnValue({ taskId: "child-2" }), + removeClineFromStack: vi.fn().mockResolvedValue(undefined), + getTaskWithId: vi.fn().mockResolvedValue({ + historyItem: makeHistoryItem({ id: "child-2", status: "active" }), + }), + updateTaskHistory: vi.fn().mockResolvedValue(undefined), + handleModeSwitch: vi.fn(), + createTask: vi.fn(), + emit: vi.fn(), + log: vi.fn(), + formatAggregatedQueueResults: (ClineProvider.prototype as any).formatAggregatedQueueResults, + } + + const subtaskQueue: SubtaskQueueItem[] = [{ mode: "code", message: "Step 1" }] + + const historyItem = makeHistoryItem({ + subtaskQueue, + subtaskQueueIndex: 0, + subtaskResults: [{ taskId: "child-1", mode: "code", summary: "Step 1 done" }], + childIds: ["child-1", "child-2"], + }) + + const result = await (ClineProvider.prototype as any).advanceSubtaskQueue.call(provider, { + parentTaskId: "parent-1", + childTaskId: "child-2", + completionResultSummary: "Step 2 done", + historyItem, + }) + + expect(result.handled).toBe(false) + expect(result.aggregatedSummary).toContain("Sequential Fan-Out Complete") + expect(result.aggregatedSummary).toContain("Step 1 done") + expect(result.aggregatedSummary).toContain("Step 2 done") + + // Should NOT have created a new child + expect(provider.createTask).not.toHaveBeenCalled() + + // Should have cleared queue from parent metadata + expect(provider.updateTaskHistory).toHaveBeenCalledWith( + expect.objectContaining({ + subtaskQueue: undefined, + subtaskQueueIndex: undefined, + }), + ) + }) + + it("returns handled=false immediately when queue is empty", async () => { + const provider = { + getCurrentTask: vi.fn(), + removeClineFromStack: vi.fn(), + getTaskWithId: vi.fn(), + updateTaskHistory: vi.fn(), + emit: vi.fn(), + log: vi.fn(), + formatAggregatedQueueResults: (ClineProvider.prototype as any).formatAggregatedQueueResults, + } + + const historyItem = makeHistoryItem({ + subtaskQueue: [], + subtaskQueueIndex: 0, + }) + + const result = await (ClineProvider.prototype as any).advanceSubtaskQueue.call(provider, { + parentTaskId: "parent-1", + childTaskId: "child-1", + completionResultSummary: "Done", + historyItem, + }) + + expect(result.handled).toBe(false) + expect(result.aggregatedSummary).toBe("Done") + }) +}) + +describe("formatAggregatedQueueResults", () => { + it("formats multiple results into a structured summary", () => { + const results = [ + { taskId: "child-1", mode: "code", summary: "Implemented feature X" }, + { taskId: "child-2", mode: "debug", summary: "Fixed bugs in feature X" }, + ] + + const formatted = (ClineProvider.prototype as any).formatAggregatedQueueResults(results, "Final result") + + expect(formatted).toContain("Sequential Fan-Out Complete (2 subtasks)") + expect(formatted).toContain("Subtask 1 (code)") + expect(formatted).toContain("Implemented feature X") + expect(formatted).toContain("Subtask 2 (debug)") + expect(formatted).toContain("Fixed bugs in feature X") + }) + + it("returns last summary when results array is empty", () => { + const formatted = (ClineProvider.prototype as any).formatAggregatedQueueResults([], "Just a summary") + expect(formatted).toBe("Just a summary") + }) + + it("handles single result", () => { + const results = [{ taskId: "child-1", mode: "code", summary: "Done" }] + const formatted = (ClineProvider.prototype as any).formatAggregatedQueueResults(results, "Done") + expect(formatted).toContain("Sequential Fan-Out Complete (1 subtask)") + expect(formatted).toContain("Subtask 1 (code)") + }) +}) diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index c391f9852cf..30bf119e4fd 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -633,6 +633,7 @@ export class NativeToolCallParser { mode: partialArgs.mode, message: partialArgs.message, todos: partialArgs.todos, + task_queue: partialArgs.task_queue, } } break @@ -982,6 +983,7 @@ export class NativeToolCallParser { mode: args.mode, message: args.message, todos: args.todos, + task_queue: args.task_queue, } as NativeArgsFor } break diff --git a/src/core/prompts/tools/native-tools/new_task.ts b/src/core/prompts/tools/native-tools/new_task.ts index f8e29e549d9..051351a9a80 100644 --- a/src/core/prompts/tools/native-tools/new_task.ts +++ b/src/core/prompts/tools/native-tools/new_task.ts @@ -2,7 +2,9 @@ import type OpenAI from "openai" const NEW_TASK_DESCRIPTION = `Create a new task instance in the chosen mode using your provided message and initial todo list (if required). -CRITICAL: This tool MUST be called alone. Do NOT call this tool alongside other tools in the same message turn. If you need to gather information before delegating, use other tools in a separate turn first, then call new_task by itself in the next turn.` +CRITICAL: This tool MUST be called alone. Do NOT call this tool alongside other tools in the same message turn. If you need to gather information before delegating, use other tools in a separate turn first, then call new_task by itself in the next turn. + +SEQUENTIAL FAN-OUT: You can optionally provide a task_queue parameter to define additional subtasks that will execute automatically in sequence after the first subtask completes. Each queued subtask runs one after another without returning to the parent in between, saving time and API calls. Use this when you have planned multiple independent subtasks upfront. The first subtask is defined by the mode and message parameters; subsequent subtasks are defined in the task_queue array.` const MODE_PARAMETER_DESCRIPTION = `Slug of the mode to begin the new task in (e.g., code, debug, architect)` @@ -10,6 +12,8 @@ const MESSAGE_PARAMETER_DESCRIPTION = `Initial user instructions or context for const TODOS_PARAMETER_DESCRIPTION = `Optional initial todo list written as a markdown checklist; required when the workspace mandates todos` +const TASK_QUEUE_PARAMETER_DESCRIPTION = `Optional JSON array of additional subtasks to execute sequentially after the first subtask completes. Each element is an object with "mode" (string) and "message" (string). Example: [{"mode":"code","message":"Implement feature X"},{"mode":"debug","message":"Test feature X"}]. When provided, the system automatically transitions between subtasks without returning to the parent, collecting all results. The parent receives aggregated results when the entire queue completes.` + export default { type: "function", function: { @@ -31,6 +35,10 @@ export default { type: ["string", "null"], description: TODOS_PARAMETER_DESCRIPTION, }, + task_queue: { + type: ["string", "null"], + description: TASK_QUEUE_PARAMETER_DESCRIPTION, + }, }, required: ["mode", "message", "todos"], additionalProperties: false, diff --git a/src/core/tools/NewTaskTool.ts b/src/core/tools/NewTaskTool.ts index f36d8e1e379..636343a489b 100644 --- a/src/core/tools/NewTaskTool.ts +++ b/src/core/tools/NewTaskTool.ts @@ -1,6 +1,7 @@ import * as vscode from "vscode" import { TodoItem } from "@roo-code/types" +import type { SubtaskQueueItem } from "@roo-code/types" import { Task } from "../task/Task" import { getModeBySlug } from "../../shared/modes" @@ -15,13 +16,14 @@ interface NewTaskParams { mode: string message: string todos?: string + task_queue?: string } export class NewTaskTool extends BaseTool<"new_task"> { readonly name = "new_task" as const async execute(params: NewTaskParams, task: Task, callbacks: ToolCallbacks): Promise { - const { mode, message, todos } = params + const { mode, message, todos, task_queue } = params const { askApproval, handleError, pushToolResult } = callbacks try { @@ -96,11 +98,47 @@ export class NewTaskTool extends BaseTool<"new_task"> { return } + // Parse task_queue if provided (sequential fan-out) + let queueItems: SubtaskQueueItem[] = [] + if (task_queue) { + try { + const parsed = JSON.parse(task_queue) + if (Array.isArray(parsed)) { + for (const item of parsed) { + if (typeof item.mode === "string" && typeof item.message === "string") { + // Validate each queued mode exists + const queuedMode = getModeBySlug(item.mode, state?.customModes) + if (!queuedMode) { + pushToolResult( + formatResponse.toolError( + `Invalid mode in task_queue: "${item.mode}". All queued subtasks must use valid modes.`, + ), + ) + return + } + queueItems.push({ mode: item.mode, message: item.message }) + } + } + } + } catch { + task.consecutiveMistakeCount++ + task.recordToolError("new_task") + task.didToolFailInCurrentTurn = true + pushToolResult( + formatResponse.toolError( + "Invalid task_queue format: must be a JSON array of objects with 'mode' and 'message' properties.", + ), + ) + return + } + } + const toolMessage = JSON.stringify({ tool: "newTask", mode: targetMode.name, content: message, todos: todoItems, + taskQueue: queueItems.length > 0 ? queueItems : undefined, }) const didApprove = await askApproval("tool", toolMessage) @@ -115,10 +153,15 @@ export class NewTaskTool extends BaseTool<"new_task"> { message: unescapedMessage, initialTodos: todoItems, mode, + subtaskQueue: queueItems.length > 0 ? queueItems : undefined, }) // Reflect delegation in tool result (no pause/unpause, no wait) - pushToolResult(`Delegated to child task ${child.taskId}`) + const queueMsg = + queueItems.length > 0 + ? ` (${queueItems.length} additional subtask${queueItems.length > 1 ? "s" : ""} queued)` + : "" + pushToolResult(`Delegated to child task ${child.taskId}${queueMsg}`) return } catch (error) { await handleError("creating new task", error) @@ -130,12 +173,14 @@ export class NewTaskTool extends BaseTool<"new_task"> { const mode: string | undefined = block.params.mode const message: string | undefined = block.params.message const todos: string | undefined = block.params.todos + const taskQueue: string | undefined = block.params.task_queue const partialMessage = JSON.stringify({ tool: "newTask", mode: mode ?? "", content: message ?? "", todos: todos, + taskQueue: taskQueue, }) await task.ask("tool", partialMessage, block.partial).catch(() => {}) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index c1b086a6a1a..67e6df1b61b 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -85,7 +85,7 @@ import { CustomModesManager } from "../config/CustomModesManager" import { Task } from "../task/Task" import { webviewMessageHandler } from "./webviewMessageHandler" -import type { ClineMessage, TodoItem } from "@roo-code/types" +import type { ClineMessage, TodoItem, SubtaskQueueItem } from "@roo-code/types" import { readApiMessages, saveApiMessages, saveTaskMessages, TaskHistoryStore } from "../task-persistence" import { readTaskMessages } from "../task-persistence/taskMessages" import { getNonce } from "./getNonce" @@ -3017,8 +3017,9 @@ export class ClineProvider message: string initialTodos: TodoItem[] mode: string + subtaskQueue?: SubtaskQueueItem[] }): Promise { - const { parentTaskId, message, initialTodos, mode } = params + const { parentTaskId, message, initialTodos, mode, subtaskQueue } = params // Metadata-driven delegation is always enabled @@ -3121,6 +3122,9 @@ export class ClineProvider delegatedToId: child.taskId, awaitingChildId: child.taskId, childIds, + ...(subtaskQueue && subtaskQueue.length > 0 + ? { subtaskQueue, subtaskQueueIndex: 0, subtaskResults: [] } + : {}), } await this.updateTaskHistory(updatedHistory) } catch (err) { @@ -3152,12 +3156,28 @@ export class ClineProvider childTaskId: string completionResultSummary: string }): Promise { - const { parentTaskId, childTaskId, completionResultSummary } = params + const { parentTaskId, childTaskId } = params + let effectiveSummary = params.completionResultSummary const globalStoragePath = this.contextProxy.globalStorageUri.fsPath // 1) Load parent from history and current persisted messages const { historyItem } = await this.getTaskWithId(parentTaskId) + // PHASE 2: Sequential fan-out — check if parent has queued subtasks + if (historyItem.subtaskQueue && historyItem.subtaskQueue.length > 0) { + const queueAdvanceResult = await this.advanceSubtaskQueue({ + parentTaskId, + childTaskId, + completionResultSummary: effectiveSummary, + historyItem, + }) + if (queueAdvanceResult.handled) { + return // Queue advanced to next subtask; do NOT reopen parent + } + // Queue exhausted — use aggregated summary and continue with normal reopen + effectiveSummary = queueAdvanceResult.aggregatedSummary + } + let parentClineMessages: ClineMessage[] = [] try { parentClineMessages = await readTaskMessages({ @@ -3203,7 +3223,7 @@ export class ClineProvider const subtaskUiMessage: ClineMessage = { type: "say", say: "subtask_result", - text: completionResultSummary, + text: effectiveSummary, ts, } parentClineMessages.push(subtaskUiMessage) @@ -3316,7 +3336,7 @@ export class ClineProvider ...historyItem, status: "active", completedByChildId: childTaskId, - completionResultSummary, + completionResultSummary: effectiveSummary, awaitingChildId: undefined, childIds, } @@ -3324,7 +3344,7 @@ export class ClineProvider // 6) Emit TaskDelegationCompleted (provider-level) try { - this.emit(RooCodeEventName.TaskDelegationCompleted, parentTaskId, childTaskId, completionResultSummary) + this.emit(RooCodeEventName.TaskDelegationCompleted, parentTaskId, childTaskId, effectiveSummary) } catch { // non-fatal } @@ -3358,6 +3378,149 @@ export class ClineProvider } } + /** + * Advance the sequential fan-out subtask queue. + * Called when a child completes and the parent has a subtaskQueue. + * + * Returns { handled: true } if the next subtask was started (caller should return). + * Returns { handled: false, aggregatedSummary } if queue is exhausted (caller should continue with normal reopen). + */ + private async advanceSubtaskQueue(params: { + parentTaskId: string + childTaskId: string + completionResultSummary: string + historyItem: HistoryItem + }): Promise<{ handled: true } | { handled: false; aggregatedSummary: string }> { + const { parentTaskId, childTaskId, completionResultSummary, historyItem } = params + const { subtaskQueue, subtaskQueueIndex, subtaskResults } = historyItem + if (!subtaskQueue || subtaskQueue.length === 0) { + return { handled: false, aggregatedSummary: completionResultSummary } + } + + const currentIndex = subtaskQueueIndex ?? 0 + const nextIndex = currentIndex + 1 + + // Record this child's result + const completedMode = historyItem.mode ?? "unknown" + const updatedResults = [ + ...(subtaskResults ?? []), + { taskId: childTaskId, mode: completedMode, summary: completionResultSummary }, + ] + + // Close current child if still open + const current = this.getCurrentTask() + if (current?.taskId === childTaskId) { + await this.removeClineFromStack() + } + + // Mark child as completed + try { + const { historyItem: childHistory } = await this.getTaskWithId(childTaskId) + await this.updateTaskHistory({ ...childHistory, status: "completed" }) + } catch (err) { + this.log( + `[advanceSubtaskQueue] Failed to persist child completed status for ${childTaskId}: ${ + (err as Error)?.message ?? String(err) + }`, + ) + } + + // Emit completion event for the finished child + try { + this.emit(RooCodeEventName.TaskDelegationCompleted, parentTaskId, childTaskId, completionResultSummary) + } catch { + // non-fatal + } + + if (nextIndex <= subtaskQueue.length - 1) { + // More subtasks in queue — start the next one + const nextSubtask = subtaskQueue[nextIndex] + this.log( + `[advanceSubtaskQueue] Auto-advancing queue: subtask ${nextIndex + 1}/${subtaskQueue.length} (mode: ${nextSubtask.mode})`, + ) + + // Switch mode + try { + await this.handleModeSwitch(nextSubtask.mode as any) + } catch (e) { + this.log( + `[advanceSubtaskQueue] handleModeSwitch failed for queued mode '${nextSubtask.mode}': ${ + (e as Error)?.message ?? String(e) + }`, + ) + } + + // Create next child + const nextChild = await this.createTask(nextSubtask.message, undefined, undefined, { + initialTodos: [], + initialStatus: "active", + startTask: false, + }) + + // Update parent metadata + const childIds = Array.from(new Set([...(historyItem.childIds ?? []), childTaskId, nextChild.taskId])) + await this.updateTaskHistory({ + ...historyItem, + status: "delegated", + delegatedToId: nextChild.taskId, + awaitingChildId: nextChild.taskId, + childIds, + subtaskQueue, + subtaskQueueIndex: nextIndex, + subtaskResults: updatedResults, + }) + + // Start the child + nextChild.start() + + try { + this.emit(RooCodeEventName.TaskDelegated, parentTaskId, nextChild.taskId) + } catch { + // non-fatal + } + + return { handled: true } + } + + // Queue exhausted — aggregate results and let normal reopen proceed + const aggregatedSummary = this.formatAggregatedQueueResults(updatedResults, completionResultSummary) + + // Clear queue from parent metadata (will be fully updated by caller) + const childIds = Array.from(new Set([...(historyItem.childIds ?? []), childTaskId])) + await this.updateTaskHistory({ + ...historyItem, + subtaskQueue: undefined, + subtaskQueueIndex: undefined, + subtaskResults: updatedResults, + childIds, + }) + + return { handled: false, aggregatedSummary } + } + + /** + * Format aggregated results from all completed subtasks in a queue. + */ + private formatAggregatedQueueResults( + results: Array<{ taskId: string; mode: string; summary: string }>, + lastSummary: string, + ): string { + if (results.length === 0) { + return lastSummary + } + + const lines = [`## Sequential Fan-Out Complete (${results.length} subtask${results.length > 1 ? "s" : ""})`, ""] + + for (let i = 0; i < results.length; i++) { + const r = results[i] + lines.push(`### Subtask ${i + 1} (${r.mode}) — ${r.taskId}`) + lines.push(r.summary) + lines.push("") + } + + return lines.join("\n") + } + /** * Convert a file path to a webview-accessible URI * This method safely converts file paths to URIs that can be loaded in the webview diff --git a/src/shared/tools.ts b/src/shared/tools.ts index 4cac8335ea7..b85639d0ae6 100644 --- a/src/shared/tools.ts +++ b/src/shared/tools.ts @@ -56,6 +56,7 @@ export const toolParamNames = [ "start_line", "end_line", "todos", + "task_queue", "prompt", "image", // read_file parameters (native protocol) @@ -102,7 +103,7 @@ export type NativeToolArgs = { edit_file: { file_path: string; old_string: string; new_string: string; expected_replacements?: number } apply_patch: { patch: string } list_files: { path: string; recursive?: boolean } - new_task: { mode: string; message: string; todos?: string } + new_task: { mode: string; message: string; todos?: string; task_queue?: string } ask_followup_question: { question: string follow_up: Array<{ text: string; mode?: string }> @@ -240,7 +241,7 @@ export interface SwitchModeToolUse extends ToolUse<"switch_mode"> { export interface NewTaskToolUse extends ToolUse<"new_task"> { name: "new_task" - params: Partial, "mode" | "message" | "todos">> + params: Partial, "mode" | "message" | "todos" | "task_queue">> } export interface RunSlashCommandToolUse extends ToolUse<"run_slash_command"> { From dcb182db93d7d809f3dd00d6fe23730305b488b4 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Tue, 12 May 2026 03:45:30 +0000 Subject: [PATCH 02/11] fix: correct queue advancement off-by-one and child mode tracking in advanceSubtaskQueue - Fix off-by-one: dispatch subtaskQueue[currentIndex] instead of subtaskQueue[nextIndex], preventing the first queued item from being skipped - Fix completedMode: fetch child history to get the child actual mode instead of incorrectly using the parent historyItem.mode - Update tests to reflect corrected queue semantics (subtaskQueueIndex represents the next item to dispatch, not the currently running item) --- src/__tests__/sequential-fan-out.spec.ts | 27 +++++++++++++--------- src/core/webview/ClineProvider.ts | 29 +++++++++++++----------- 2 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/__tests__/sequential-fan-out.spec.ts b/src/__tests__/sequential-fan-out.spec.ts index 44d7a4d41e4..792278caee3 100644 --- a/src/__tests__/sequential-fan-out.spec.ts +++ b/src/__tests__/sequential-fan-out.spec.ts @@ -64,7 +64,7 @@ describe("advanceSubtaskQueue", () => { getCurrentTask: vi.fn().mockReturnValue({ taskId: "child-1" }), removeClineFromStack: vi.fn().mockResolvedValue(undefined), getTaskWithId: vi.fn().mockResolvedValue({ - historyItem: makeHistoryItem({ id: "child-1", status: "active" }), + historyItem: makeHistoryItem({ id: "child-1", mode: "code", status: "active" }), }), updateTaskHistory: vi.fn().mockResolvedValue(undefined), handleModeSwitch: vi.fn().mockResolvedValue(undefined), @@ -73,6 +73,8 @@ describe("advanceSubtaskQueue", () => { log: vi.fn(), } + // Queue items represent ADDITIONAL subtasks after the initial child. + // subtaskQueueIndex=0 means queue[0] is the next to dispatch. const subtaskQueue: SubtaskQueueItem[] = [ { mode: "code", message: "Step 1" }, { mode: "debug", message: "Step 2" }, @@ -88,7 +90,7 @@ describe("advanceSubtaskQueue", () => { const result = await (ClineProvider.prototype as any).advanceSubtaskQueue.call(provider, { parentTaskId: "parent-1", childTaskId: "child-1", - completionResultSummary: "Step 1 done", + completionResultSummary: "Initial task done", historyItem, }) @@ -102,11 +104,11 @@ describe("advanceSubtaskQueue", () => { expect.objectContaining({ id: "child-1", status: "completed" }), ) - // Should have switched mode to next subtask's mode - expect(provider.handleModeSwitch).toHaveBeenCalledWith("debug") + // Should have switched mode to queue[0]'s mode (the next item to dispatch) + expect(provider.handleModeSwitch).toHaveBeenCalledWith("code") - // Should have created the next child with the queued message - expect(provider.createTask).toHaveBeenCalledWith("Step 2", undefined, undefined, { + // Should have created the next child with queue[0]'s message + expect(provider.createTask).toHaveBeenCalledWith("Step 1", undefined, undefined, { initialTodos: [], initialStatus: "active", startTask: false, @@ -115,12 +117,13 @@ describe("advanceSubtaskQueue", () => { // Should have started the next child expect(mockChild.start).toHaveBeenCalled() - // Should have updated parent with advanced queue index + // Should have updated parent with advanced queue index (0 -> 1) + // completedMode comes from child's history (mode: "code") expect(provider.updateTaskHistory).toHaveBeenCalledWith( expect.objectContaining({ id: "parent-1", subtaskQueueIndex: 1, - subtaskResults: [{ taskId: "child-1", mode: "unknown", summary: "Step 1 done" }], + subtaskResults: [{ taskId: "child-1", mode: "code", summary: "Initial task done" }], awaitingChildId: "child-2", delegatedToId: "child-2", }), @@ -131,7 +134,7 @@ describe("advanceSubtaskQueue", () => { RooCodeEventName.TaskDelegationCompleted, "parent-1", "child-1", - "Step 1 done", + "Initial task done", ) expect(emitSpy).toHaveBeenCalledWith(RooCodeEventName.TaskDelegated, "parent-1", "child-2") }) @@ -141,7 +144,7 @@ describe("advanceSubtaskQueue", () => { getCurrentTask: vi.fn().mockReturnValue({ taskId: "child-2" }), removeClineFromStack: vi.fn().mockResolvedValue(undefined), getTaskWithId: vi.fn().mockResolvedValue({ - historyItem: makeHistoryItem({ id: "child-2", status: "active" }), + historyItem: makeHistoryItem({ id: "child-2", mode: "code", status: "active" }), }), updateTaskHistory: vi.fn().mockResolvedValue(undefined), handleModeSwitch: vi.fn(), @@ -151,11 +154,13 @@ describe("advanceSubtaskQueue", () => { formatAggregatedQueueResults: (ClineProvider.prototype as any).formatAggregatedQueueResults, } + // Queue has 1 item, subtaskQueueIndex=1 means queue[0] was already dispatched. + // Now that child completes and the queue is exhausted. const subtaskQueue: SubtaskQueueItem[] = [{ mode: "code", message: "Step 1" }] const historyItem = makeHistoryItem({ subtaskQueue, - subtaskQueueIndex: 0, + subtaskQueueIndex: 1, subtaskResults: [{ taskId: "child-1", mode: "code", summary: "Step 1 done" }], childIds: ["child-1", "child-2"], }) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 67e6df1b61b..6810bdb2477 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -3397,15 +3397,10 @@ export class ClineProvider return { handled: false, aggregatedSummary: completionResultSummary } } + // currentIndex is the next queue item to dispatch (0-based). + // When the initial child (from mode/message params) completes, currentIndex is 0, + // meaning queue[0] should be dispatched first. const currentIndex = subtaskQueueIndex ?? 0 - const nextIndex = currentIndex + 1 - - // Record this child's result - const completedMode = historyItem.mode ?? "unknown" - const updatedResults = [ - ...(subtaskResults ?? []), - { taskId: childTaskId, mode: completedMode, summary: completionResultSummary }, - ] // Close current child if still open const current = this.getCurrentTask() @@ -3413,9 +3408,11 @@ export class ClineProvider await this.removeClineFromStack() } - // Mark child as completed + // Fetch child history to get the child's actual mode and mark it completed + let completedMode = "unknown" try { const { historyItem: childHistory } = await this.getTaskWithId(childTaskId) + completedMode = childHistory.mode ?? "unknown" await this.updateTaskHistory({ ...childHistory, status: "completed" }) } catch (err) { this.log( @@ -3425,6 +3422,12 @@ export class ClineProvider ) } + // Record this child's result using the child's actual mode + const updatedResults = [ + ...(subtaskResults ?? []), + { taskId: childTaskId, mode: completedMode, summary: completionResultSummary }, + ] + // Emit completion event for the finished child try { this.emit(RooCodeEventName.TaskDelegationCompleted, parentTaskId, childTaskId, completionResultSummary) @@ -3432,11 +3435,11 @@ export class ClineProvider // non-fatal } - if (nextIndex <= subtaskQueue.length - 1) { + if (currentIndex < subtaskQueue.length) { // More subtasks in queue — start the next one - const nextSubtask = subtaskQueue[nextIndex] + const nextSubtask = subtaskQueue[currentIndex] this.log( - `[advanceSubtaskQueue] Auto-advancing queue: subtask ${nextIndex + 1}/${subtaskQueue.length} (mode: ${nextSubtask.mode})`, + `[advanceSubtaskQueue] Auto-advancing queue: subtask ${currentIndex + 1}/${subtaskQueue.length} (mode: ${nextSubtask.mode})`, ) // Switch mode @@ -3466,7 +3469,7 @@ export class ClineProvider awaitingChildId: nextChild.taskId, childIds, subtaskQueue, - subtaskQueueIndex: nextIndex, + subtaskQueueIndex: currentIndex + 1, subtaskResults: updatedResults, }) From 3fd128832c9dd928e3b7c82faf1f0e8d22ee7ce2 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Tue, 12 May 2026 04:03:56 +0000 Subject: [PATCH 03/11] fix: add retry logic to copyDir/copyPaths for EBUSY errors on Windows The Windows CI bundle step fails with EBUSY when antivirus or indexing services hold brief locks on files during copyFileSync. Add a copyFileWithRetry helper (matching the existing rmDir retry pattern) that retries up to 5 times with exponential backoff for EBUSY, EPERM, and EACCES errors. --- packages/build/src/esbuild.ts | 40 +++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/packages/build/src/esbuild.ts b/packages/build/src/esbuild.ts index 952e823eeca..c4898015128 100644 --- a/packages/build/src/esbuild.ts +++ b/packages/build/src/esbuild.ts @@ -4,6 +4,42 @@ import { execSync } from "child_process" import { ViewsContainer, Views, Menus, Configuration, Keybindings, contributesSchema } from "./types.js" +/** + * Copy a single file with retry logic to handle transient Windows file-locking + * errors (EBUSY, EPERM, EACCES) that occur when antivirus or indexing services + * hold brief locks on files during CI builds. + */ +function copyFileWithRetry(src: string, dst: string, maxRetries: number = 5): void { + for (let attempt = 1; attempt <= maxRetries; attempt++) { + try { + fs.copyFileSync(src, dst) + return + } catch (error) { + const isRetryable = + error instanceof Error && + "code" in error && + ((error as NodeJS.ErrnoException).code === "EBUSY" || + (error as NodeJS.ErrnoException).code === "EPERM" || + (error as NodeJS.ErrnoException).code === "EACCES") + + if (!isRetryable || attempt === maxRetries) { + throw error + } + + const baseDelay = process.platform === "win32" ? 200 : 100 + const delay = Math.min(baseDelay * Math.pow(2, attempt - 1), 2000) + console.warn(`[copyFileWithRetry] Attempt ${attempt} failed for ${src}, retrying in ${delay}ms...`) + + // Synchronous sleep (same pattern as rmDir). + const start = Date.now() + + while (Date.now() - start < delay) { + /* Busy wait */ + } + } + } +} + function copyDir(srcDir: string, dstDir: string, count: number): number { const entries = fs.readdirSync(srcDir, { withFileTypes: true }) @@ -16,7 +52,7 @@ function copyDir(srcDir: string, dstDir: string, count: number): number { count = copyDir(srcPath, dstPath, count) } else { count = count + 1 - fs.copyFileSync(srcPath, dstPath) + copyFileWithRetry(srcPath, dstPath) } } @@ -98,7 +134,7 @@ export function copyPaths(copyPaths: [string, string, CopyPathOptions?][], srcDi const count = copyDir(path.join(srcDir, srcRelPath), path.join(dstDir, dstRelPath), 0) console.log(`[copyPaths] Copied ${count} files from ${srcRelPath} to ${dstRelPath}`) } else { - fs.copyFileSync(path.join(srcDir, srcRelPath), path.join(dstDir, dstRelPath)) + copyFileWithRetry(path.join(srcDir, srcRelPath), path.join(dstDir, dstRelPath)) console.log(`[copyPaths] Copied ${srcRelPath} to ${dstRelPath}`) } } catch (error) { From 16de6be4ea3dbd14dc8a68b5c150864a065f1b77 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Tue, 12 May 2026 05:00:07 +0000 Subject: [PATCH 04/11] feat: add TaskContext and TaskPermissions for Phase 3a task isolation Introduces the foundation for isolated task execution (Phase 3a of #12330): - TaskContext: immutable snapshot of mode, API config, and workspace for each task, replacing runtime reads from shared ClineProvider state - TaskPermissions: fine-grained permission boundaries (file patterns, command restrictions, read-only mode, tool allowlists) that the orchestrator can attach when spawning subtasks - TaskContextBuilder: factory functions to build TaskContext from provider state and to derive child contexts with merged permissions - Task constructor now accepts optional taskContext, using it for mode and API config initialization instead of provider.getState() - delegateParentAndOpenChild builds and passes a TaskContext to child tasks - Permission merging follows most-restrictive-wins semantics This is a pure refactor with no behavioral change -- tasks still execute sequentially, but they now carry their own isolated context. Enforcement of permission boundaries is deferred to Phase 3b/3d. Ref: #12330 --- .../types/src/__tests__/task-context.spec.ts | 126 ++++++++++ packages/types/src/index.ts | 1 + packages/types/src/task-context.ts | 227 ++++++++++++++++++ packages/types/src/task.ts | 7 + src/__tests__/provider-delegation.spec.ts | 3 + src/__tests__/task-context-builder.spec.ts | 130 ++++++++++ src/core/task/Task.ts | 29 +++ src/core/task/TaskContextBuilder.ts | 63 +++++ src/core/webview/ClineProvider.ts | 30 ++- 9 files changed, 608 insertions(+), 8 deletions(-) create mode 100644 packages/types/src/__tests__/task-context.spec.ts create mode 100644 packages/types/src/task-context.ts create mode 100644 src/__tests__/task-context-builder.spec.ts create mode 100644 src/core/task/TaskContextBuilder.ts diff --git a/packages/types/src/__tests__/task-context.spec.ts b/packages/types/src/__tests__/task-context.spec.ts new file mode 100644 index 00000000000..0e28d23dff7 --- /dev/null +++ b/packages/types/src/__tests__/task-context.spec.ts @@ -0,0 +1,126 @@ +import { describe, it, expect } from "vitest" + +import { + taskPermissionsSchema, + taskContextSchema, + mergePermissions, + type TaskPermissions, + type TaskContext, +} from "../task-context.js" + +describe("TaskPermissions schema", () => { + it("accepts empty object", () => { + const result = taskPermissionsSchema.parse({}) + expect(result).toEqual({}) + }) + + it("accepts full permissions object", () => { + const permissions: TaskPermissions = { + fileReadPatterns: ["docs/**", "src/**"], + fileWritePatterns: ["docs/**"], + allowedCommands: ["npm test"], + blockedCommands: ["rm -rf"], + readOnly: true, + allowedTools: ["read_file", "list_files"], + } + const result = taskPermissionsSchema.parse(permissions) + expect(result).toEqual(permissions) + }) + + it("accepts partial permissions", () => { + const result = taskPermissionsSchema.parse({ readOnly: true }) + expect(result).toEqual({ readOnly: true }) + }) + + it("rejects invalid types", () => { + expect(() => taskPermissionsSchema.parse({ readOnly: "yes" })).toThrow() + expect(() => taskPermissionsSchema.parse({ fileReadPatterns: "docs/**" })).toThrow() + }) +}) + +describe("TaskContext schema", () => { + it("accepts minimal context", () => { + const context: TaskContext = { mode: "code" } + const result = taskContextSchema.parse(context) + expect(result.mode).toBe("code") + }) + + it("accepts full context", () => { + const context: TaskContext = { + mode: "architect", + apiConfigName: "gpt-4", + permissions: { + readOnly: true, + fileReadPatterns: ["docs/**"], + }, + inheritSkills: true, + skillOverrides: ["custom-skill"], + workspacePath: "/workspace/project", + parentTaskId: "parent-123", + rootTaskId: "root-456", + } + const result = taskContextSchema.parse(context) + expect(result).toEqual(context) + }) + + it("rejects missing mode", () => { + expect(() => taskContextSchema.parse({})).toThrow() + }) +}) + +describe("mergePermissions", () => { + it("returns undefined when both are undefined", () => { + expect(mergePermissions(undefined, undefined)).toBeUndefined() + }) + + it("returns child when parent is undefined", () => { + const child: TaskPermissions = { readOnly: true } + expect(mergePermissions(undefined, child)).toEqual(child) + }) + + it("returns parent when child is undefined", () => { + const parent: TaskPermissions = { readOnly: true } + expect(mergePermissions(parent, undefined)).toEqual(parent) + }) + + it("merges readOnly with OR logic", () => { + expect(mergePermissions({ readOnly: true }, { readOnly: false })).toMatchObject({ readOnly: true }) + expect(mergePermissions({ readOnly: false }, { readOnly: true })).toMatchObject({ readOnly: true }) + expect(mergePermissions({ readOnly: false }, { readOnly: false })).toMatchObject({}) + }) + + it("intersects fileWritePatterns", () => { + const parent: TaskPermissions = { fileWritePatterns: ["docs/**", "src/**", "package.json"] } + const child: TaskPermissions = { fileWritePatterns: ["docs/**", "package.json"] } + const result = mergePermissions(parent, child) + expect(result?.fileWritePatterns).toEqual(["docs/**", "package.json"]) + }) + + it("intersects allowedTools", () => { + const parent: TaskPermissions = { allowedTools: ["read_file", "list_files", "search_files"] } + const child: TaskPermissions = { allowedTools: ["read_file", "search_files", "write_to_file"] } + const result = mergePermissions(parent, child) + expect(result?.allowedTools).toEqual(["read_file", "search_files"]) + }) + + it("unions blockedCommands", () => { + const parent: TaskPermissions = { blockedCommands: ["rm -rf"] } + const child: TaskPermissions = { blockedCommands: ["git push", "rm -rf"] } + const result = mergePermissions(parent, child) + expect(result?.blockedCommands).toEqual(["rm -rf", "git push"]) + }) + + it("returns defined array when only one side specifies it", () => { + const parent: TaskPermissions = { fileReadPatterns: ["docs/**"] } + const child: TaskPermissions = {} + const result = mergePermissions(parent, child) + expect(result?.fileReadPatterns).toEqual(["docs/**"]) + }) + + it("returns empty array when intersection is empty", () => { + const parent: TaskPermissions = { allowedTools: ["read_file"] } + const child: TaskPermissions = { allowedTools: ["write_to_file"] } + const result = mergePermissions(parent, child) + expect(result?.allowedTools).toEqual([]) + }) +}) diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index f3723bc68a1..d147f2d94d5 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -20,6 +20,7 @@ export * from "./mode.js" export * from "./model.js" export * from "./provider-settings.js" export * from "./task.js" +export * from "./task-context.js" export * from "./todo.js" export * from "./skills.js" export * from "./terminal.js" diff --git a/packages/types/src/task-context.ts b/packages/types/src/task-context.ts new file mode 100644 index 00000000000..7040f0297b9 --- /dev/null +++ b/packages/types/src/task-context.ts @@ -0,0 +1,227 @@ +import { z } from "zod" + +/** + * TaskPermissions defines fine-grained permission boundaries for a subtask. + * + * These permissions allow the orchestrator (or parent task) to restrict what + * a child task can do, making parallel execution safer by preventing + * unintended side effects across task boundaries. + * + * ## Design Notes + * + * Phase 3a introduces the types and plumbing. Enforcement is deferred to + * Phase 3b (read-only parallelism) and Phase 3d (write parallelism). + * + * The permission model is intentionally additive: if no permissions are + * specified, the task inherits full capabilities from its mode. Permissions + * can only *restrict*, never *expand* beyond what the mode allows. + */ +export const taskPermissionsSchema = z.object({ + /** + * Glob patterns restricting which files the task may read. + * If empty or undefined, the task can read any file (subject to mode restrictions). + * Examples: ["docs/**", "src/utils/**"] + */ + fileReadPatterns: z.array(z.string()).optional(), + + /** + * Glob patterns restricting which files the task may write/edit. + * If empty or undefined, the task can write any file (subject to mode restrictions). + * Examples: ["docs/**", "package.json"] + */ + fileWritePatterns: z.array(z.string()).optional(), + + /** + * Allowlist of shell commands the task may execute. + * If empty or undefined, the task can execute any command (subject to mode restrictions). + * Matched as prefixes against the command string. + * Examples: ["npm test", "npx vitest", "git status"] + */ + allowedCommands: z.array(z.string()).optional(), + + /** + * Blocklist of shell commands the task may NOT execute. + * Takes precedence over allowedCommands. + * Examples: ["rm -rf", "git push"] + */ + blockedCommands: z.array(z.string()).optional(), + + /** + * Whether the task is restricted to read-only operations. + * When true, the task cannot use write tools (write_to_file, apply_diff, + * execute_command, etc.). This is the primary mechanism for Phase 3b + * read-only parallelism. + */ + readOnly: z.boolean().optional(), + + /** + * Explicit list of tool names the task is allowed to use. + * If empty or undefined, all tools available to the mode are allowed. + * Examples: ["read_file", "list_files", "search_files"] + */ + allowedTools: z.array(z.string()).optional(), +}) + +export type TaskPermissions = z.infer + +/** + * TaskContext encapsulates all per-task configuration that a Task needs + * to operate independently of the ClineProvider's shared mutable state. + * + * ## Purpose + * + * Today, Task reads mode, API config, and other settings from the provider + * via `provider.getState()` at construction time and during execution. + * This couples Task execution to the provider's current state, which + * prevents multiple tasks from running concurrently (since they'd all + * read/write the same shared state). + * + * TaskContext captures a snapshot of everything a Task needs at creation + * time, so the Task can operate with its own isolated configuration. + * + * ## Lifecycle + * + * 1. Built by the parent (orchestrator or provider) when creating a subtask + * 2. Passed to the Task constructor as an immutable snapshot + * 3. The Task uses this context instead of reaching back to the provider + * for mode/config/permissions during execution + * + * ## Phase 3a Scope + * + * In Phase 3a, TaskContext is optional -- tasks that don't receive one + * fall back to the existing provider.getState() behavior. This ensures + * full backward compatibility while enabling incremental adoption. + */ +export const taskContextSchema = z.object({ + /** + * The mode slug for this task (e.g., "code", "architect", "ask"). + * Snapshot at task creation time -- does not change if the provider's + * mode changes later. + */ + mode: z.string(), + + /** + * The API configuration profile name for this task. + * Allows subtasks to use different models (including local ones) + * from the parent task. + */ + apiConfigName: z.string().optional(), + + /** + * Permission boundaries for this task. + * If undefined, the task inherits full capabilities from its mode. + */ + permissions: taskPermissionsSchema.optional(), + + /** + * Whether this task should inherit skills from the parent. + * Defaults to true if not specified. + */ + inheritSkills: z.boolean().optional(), + + /** + * Additional skill overrides for this task. + * These are merged with (or replace) inherited skills depending + * on the inheritSkills setting. + */ + skillOverrides: z.array(z.string()).optional(), + + /** + * The workspace path for this task. + * Allows subtasks to operate in different workspace roots. + */ + workspacePath: z.string().optional(), + + /** + * ID of the parent task that created this context. + * Used for lineage tracking and result aggregation. + */ + parentTaskId: z.string().optional(), + + /** + * ID of the root task in the delegation chain. + * Used for hierarchical task management. + */ + rootTaskId: z.string().optional(), +}) + +export type TaskContext = z.infer + +/** + * Merge two TaskPermissions objects, producing the most restrictive + * combination. This is used when a parent task's permissions should + * further constrain a child task's permissions. + * + * Rules: + * - readOnly: true if either is true + * - allowedTools: intersection if both specified, otherwise the one that's specified + * - fileReadPatterns / fileWritePatterns: intersection if both specified + * - allowedCommands: intersection if both specified + * - blockedCommands: union (all blocked commands from both) + */ +export function mergePermissions( + parent: TaskPermissions | undefined, + child: TaskPermissions | undefined, +): TaskPermissions | undefined { + if (!parent && !child) { + return undefined + } + + if (!parent) { + return child + } + + if (!child) { + return parent + } + + return { + readOnly: parent.readOnly || child.readOnly || undefined, + + fileReadPatterns: intersectArrays(parent.fileReadPatterns, child.fileReadPatterns), + + fileWritePatterns: intersectArrays(parent.fileWritePatterns, child.fileWritePatterns), + + allowedCommands: intersectArrays(parent.allowedCommands, child.allowedCommands), + + blockedCommands: unionArrays(parent.blockedCommands, child.blockedCommands), + + allowedTools: intersectArrays(parent.allowedTools, child.allowedTools), + } +} + +/** Return intersection of two optional arrays, or the defined one if only one exists. */ +function intersectArrays(a: string[] | undefined, b: string[] | undefined): string[] | undefined { + if (!a && !b) { + return undefined + } + + if (!a) { + return b + } + + if (!b) { + return a + } + + const setB = new Set(b) + const result = a.filter((item) => setB.has(item)) + return result.length > 0 ? result : [] +} + +/** Return union of two optional arrays. */ +function unionArrays(a: string[] | undefined, b: string[] | undefined): string[] | undefined { + if (!a && !b) { + return undefined + } + + if (!a) { + return b + } + + if (!b) { + return a + } + + return Array.from(new Set([...a, ...b])) +} diff --git a/packages/types/src/task.ts b/packages/types/src/task.ts index 7447dc772e7..e664714a3a5 100644 --- a/packages/types/src/task.ts +++ b/packages/types/src/task.ts @@ -5,6 +5,7 @@ import type { RooCodeSettings } from "./global-settings.js" import type { ClineMessage, QueuedMessage, TokenUsage } from "./message.js" import type { ToolUsage, ToolName } from "./tool.js" import type { TodoItem } from "./todo.js" +import type { TaskContext } from "./task-context.js" /** * TaskProviderLike @@ -94,6 +95,12 @@ export interface CreateTaskOptions { /** Whether to start the task loop immediately (default: true). * When false, the caller must invoke `task.start()` manually. */ startTask?: boolean + /** + * Optional isolated task context containing mode, API config, and permissions. + * When provided, the task uses this context instead of reading from the provider. + * Phase 3a foundation for concurrent task execution. + */ + taskContext?: TaskContext } export enum TaskStatus { diff --git a/src/__tests__/provider-delegation.spec.ts b/src/__tests__/provider-delegation.spec.ts index 4b04fb5bbb9..ef2b0b12104 100644 --- a/src/__tests__/provider-delegation.spec.ts +++ b/src/__tests__/provider-delegation.spec.ts @@ -47,6 +47,7 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => { getTaskWithId, updateTaskHistory, handleModeSwitch, + getState: vi.fn().mockResolvedValue({ mode: "code", currentApiConfigName: "default" }), log: vi.fn(), } as unknown as ClineProvider @@ -68,6 +69,7 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => { initialTodos: [], initialStatus: "active", startTask: false, + taskContext: expect.objectContaining({ mode: "code" }), }) // Metadata persistence - parent gets "delegated" status (child status is set at creation via initialStatus) @@ -129,6 +131,7 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => { getTaskWithId, updateTaskHistory, handleModeSwitch, + getState: vi.fn().mockResolvedValue({ mode: "code", currentApiConfigName: "default" }), log: vi.fn(), } as unknown as ClineProvider diff --git a/src/__tests__/task-context-builder.spec.ts b/src/__tests__/task-context-builder.spec.ts new file mode 100644 index 00000000000..ccf3b9e6841 --- /dev/null +++ b/src/__tests__/task-context-builder.spec.ts @@ -0,0 +1,130 @@ +import { describe, it, expect, vi } from "vitest" + +import { buildTaskContext, buildChildTaskContext } from "../core/task/TaskContextBuilder" +import { defaultModeSlug } from "../shared/modes" +import type { TaskContext } from "@roo-code/types" + +describe("buildTaskContext", () => { + it("snapshots mode and API config from provider state", async () => { + const provider = { + getState: vi.fn().mockResolvedValue({ + mode: "architect", + currentApiConfigName: "gpt-4-profile", + }), + } as any + + const ctx = await buildTaskContext(provider) + expect(ctx.mode).toBe("architect") + expect(ctx.apiConfigName).toBe("gpt-4-profile") + expect(ctx.inheritSkills).toBe(true) + }) + + it("applies overrides over provider state", async () => { + const provider = { + getState: vi.fn().mockResolvedValue({ + mode: "code", + currentApiConfigName: "default", + }), + } as any + + const ctx = await buildTaskContext(provider, { + mode: "ask", + apiConfigName: "local-model", + permissions: { readOnly: true }, + parentTaskId: "parent-1", + }) + + expect(ctx.mode).toBe("ask") + expect(ctx.apiConfigName).toBe("local-model") + expect(ctx.permissions?.readOnly).toBe(true) + expect(ctx.parentTaskId).toBe("parent-1") + }) + + it("falls back to defaults when provider state is empty", async () => { + const provider = { + getState: vi.fn().mockResolvedValue(null), + } as any + + const ctx = await buildTaskContext(provider) + expect(ctx.mode).toBe(defaultModeSlug) + expect(ctx.apiConfigName).toBe("default") + }) +}) + +describe("buildChildTaskContext", () => { + it("inherits parent context when no overrides", () => { + const parent: TaskContext = { + mode: "orchestrator", + apiConfigName: "gpt-4", + permissions: { fileWritePatterns: ["docs/**"] }, + inheritSkills: true, + workspacePath: "/workspace", + rootTaskId: "root-1", + } + + const child = buildChildTaskContext(parent, { parentTaskId: "parent-1" }) + + expect(child.mode).toBe("orchestrator") + expect(child.apiConfigName).toBe("gpt-4") + expect(child.permissions?.fileWritePatterns).toEqual(["docs/**"]) + expect(child.workspacePath).toBe("/workspace") + expect(child.rootTaskId).toBe("root-1") + expect(child.parentTaskId).toBe("parent-1") + }) + + it("overrides mode and API config for child", () => { + const parent: TaskContext = { + mode: "orchestrator", + apiConfigName: "gpt-4", + rootTaskId: "root-1", + } + + const child = buildChildTaskContext(parent, { + mode: "code", + apiConfigName: "local-llama", + parentTaskId: "parent-1", + }) + + expect(child.mode).toBe("code") + expect(child.apiConfigName).toBe("local-llama") + expect(child.rootTaskId).toBe("root-1") + }) + + it("merges permissions using most-restrictive rule", () => { + const parent: TaskContext = { + mode: "orchestrator", + permissions: { + fileWritePatterns: ["docs/**", "src/**"], + allowedTools: ["read_file", "write_to_file", "list_files"], + }, + } + + const child = buildChildTaskContext(parent, { + mode: "code", + permissions: { + fileWritePatterns: ["docs/**"], + allowedTools: ["read_file", "list_files"], + }, + parentTaskId: "parent-1", + }) + + // Intersection of file write patterns + expect(child.permissions?.fileWritePatterns).toEqual(["docs/**"]) + // Intersection of allowed tools + expect(child.permissions?.allowedTools).toEqual(["read_file", "list_files"]) + }) + + it("inherits parent permissions when child specifies none", () => { + const parent: TaskContext = { + mode: "orchestrator", + permissions: { readOnly: true }, + } + + const child = buildChildTaskContext(parent, { + mode: "ask", + parentTaskId: "parent-1", + }) + + expect(child.permissions?.readOnly).toBe(true) + }) +}) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 97f07fcc7aa..fe5241188fc 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -51,6 +51,7 @@ import { MIN_CHECKPOINT_TIMEOUT_SECONDS, MAX_MCP_TOOLS_THRESHOLD, countEnabledMcpTools, + type TaskContext, } from "@roo-code/types" // api @@ -153,6 +154,15 @@ export interface TaskOptions extends CreateTaskOptions { workspacePath?: string /** Initial status for the task's history item (e.g., "active" for child tasks) */ initialStatus?: "active" | "delegated" | "completed" + /** + * Optional isolated task context containing mode, API config, and permissions. + * When provided, the task uses this context instead of reading from the provider. + * This is the foundation for Phase 3a task isolation -- tasks that carry their + * own context can eventually run concurrently without shared state conflicts. + * + * If not provided, the task falls back to the existing provider.getState() behavior. + */ + taskContext?: TaskContext } export class Task extends EventEmitter implements TaskLike { @@ -172,6 +182,15 @@ export class Task extends EventEmitter implements TaskLike { readonly taskNumber: number readonly workspacePath: string + /** + * Isolated task context carrying mode, API config, and permission boundaries. + * When set, the task uses this context instead of reading shared provider state. + * This is the foundation for concurrent task execution in later phases. + * + * @see TaskContext in @roo-code/types + */ + readonly taskContext?: TaskContext + /** * The mode associated with this task. Persisted across sessions * to maintain user context when reopening tasks from history. @@ -430,6 +449,7 @@ export class Task extends EventEmitter implements TaskLike { initialTodos, workspacePath, initialStatus, + taskContext, }: TaskOptions) { super() @@ -491,6 +511,7 @@ export class Task extends EventEmitter implements TaskLike { this.parentTask = parentTask this.taskNumber = taskNumber this.initialStatus = initialStatus + this.taskContext = taskContext this.assistantMessageParser = undefined @@ -544,6 +565,14 @@ export class Task extends EventEmitter implements TaskLike { this._taskApiConfigName = historyItem.apiConfigName this.taskModeReady = Promise.resolve() this.taskApiConfigReady = Promise.resolve() + } else if (taskContext) { + // Phase 3a: Use isolated TaskContext instead of reading from provider state. + // This allows the task to carry its own mode and API config snapshot, + // independent of the provider's shared mutable state. + this._taskMode = taskContext.mode || defaultModeSlug + this._taskApiConfigName = taskContext.apiConfigName ?? "default" + this.taskModeReady = Promise.resolve() + this.taskApiConfigReady = Promise.resolve() } else { this._taskMode = undefined this._taskApiConfigName = undefined diff --git a/src/core/task/TaskContextBuilder.ts b/src/core/task/TaskContextBuilder.ts new file mode 100644 index 00000000000..dbcf88f87bc --- /dev/null +++ b/src/core/task/TaskContextBuilder.ts @@ -0,0 +1,63 @@ +import { type TaskContext, type TaskPermissions, mergePermissions } from "@roo-code/types" + +import { defaultModeSlug } from "../../shared/modes" +import type { ClineProvider } from "../webview/ClineProvider" + +/** + * Build a TaskContext from the current provider state. + * + * This factory snapshots the provider's current mode and API config + * into an immutable TaskContext that a child task can carry independently. + * This is the key enabler for Phase 3a: tasks no longer need to reach + * back into the provider for their mode/config during execution. + * + * @param provider - The ClineProvider to snapshot state from + * @param overrides - Optional overrides (e.g., mode from new_task tool) + * @returns A TaskContext snapshot + */ +export async function buildTaskContext( + provider: ClineProvider, + overrides?: Partial, +): Promise { + const state = await provider.getState() + + const context: TaskContext = { + mode: overrides?.mode ?? state?.mode ?? defaultModeSlug, + apiConfigName: overrides?.apiConfigName ?? state?.currentApiConfigName ?? "default", + permissions: overrides?.permissions, + inheritSkills: overrides?.inheritSkills ?? true, + skillOverrides: overrides?.skillOverrides, + workspacePath: overrides?.workspacePath, + parentTaskId: overrides?.parentTaskId, + rootTaskId: overrides?.rootTaskId, + } + + return context +} + +/** + * Build a TaskContext for a child task, inheriting from a parent context + * and applying any child-specific overrides. + * + * Permission merging follows the "most restrictive" principle: + * the child's effective permissions are the intersection of the parent's + * permissions and any child-specific permissions. + * + * @param parentContext - The parent task's context + * @param childOverrides - Child-specific overrides + * @returns A new TaskContext for the child task + */ +export function buildChildTaskContext(parentContext: TaskContext, childOverrides: Partial): TaskContext { + const mergedPermissions = mergePermissions(parentContext.permissions, childOverrides.permissions) + + return { + mode: childOverrides.mode ?? parentContext.mode, + apiConfigName: childOverrides.apiConfigName ?? parentContext.apiConfigName, + permissions: mergedPermissions, + inheritSkills: childOverrides.inheritSkills ?? parentContext.inheritSkills, + skillOverrides: childOverrides.skillOverrides ?? parentContext.skillOverrides, + workspacePath: childOverrides.workspacePath ?? parentContext.workspacePath, + parentTaskId: childOverrides.parentTaskId, + rootTaskId: childOverrides.rootTaskId ?? parentContext.rootTaskId, + } +} diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 6810bdb2477..ea7f1f27d27 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -83,9 +83,10 @@ import { ContextProxy } from "../config/ContextProxy" import { ProviderSettingsManager } from "../config/ProviderSettingsManager" import { CustomModesManager } from "../config/CustomModesManager" import { Task } from "../task/Task" +import { buildTaskContext } from "../task/TaskContextBuilder" import { webviewMessageHandler } from "./webviewMessageHandler" -import type { ClineMessage, TodoItem, SubtaskQueueItem } from "@roo-code/types" +import type { ClineMessage, TodoItem, SubtaskQueueItem, TaskPermissions } from "@roo-code/types" import { readApiMessages, saveApiMessages, saveTaskMessages, TaskHistoryStore } from "../task-persistence" import { readTaskMessages } from "../task-persistence/taskMessages" import { getNonce } from "./getNonce" @@ -3018,8 +3019,10 @@ export class ClineProvider initialTodos: TodoItem[] mode: string subtaskQueue?: SubtaskQueueItem[] + /** Optional permission boundaries for the child task (Phase 3a) */ + permissions?: TaskPermissions }): Promise { - const { parentTaskId, message, initialTodos, mode, subtaskQueue } = params + const { parentTaskId, message, initialTodos, mode, subtaskQueue, permissions } = params // Metadata-driven delegation is always enabled @@ -3095,24 +3098,35 @@ export class ClineProvider ) } - // 4) Create child as sole active (parent reference preserved for lineage) + // 4) Build an isolated TaskContext for the child (Phase 3a). + // This snapshots mode, API config, and permission boundaries so the child + // task carries its own context instead of reading shared provider state. + const childTaskContext = await buildTaskContext(this, { + mode, + permissions, + parentTaskId, + rootTaskId: parent.rootTaskId ?? parent.taskId, + }) + + // 5) Create child as sole active (parent reference preserved for lineage) // Pass initialStatus: "active" to ensure the child task's historyItem is created // with status from the start, avoiding race conditions where the task might // call attempt_completion before status is persisted separately. // // Pass startTask: false to prevent the child from beginning its task loop // (and writing to globalState via saveClineMessages → updateTaskHistory) - // before we persist the parent's delegation metadata in step 5. - // Without this, the child's fire-and-forget startTask() races with step 5, + // before we persist the parent's delegation metadata in step 6. + // Without this, the child's fire-and-forget startTask() races with step 6, // and the last writer to globalState overwrites the other's changes— // causing the parent's delegation fields to be lost. const child = await this.createTask(message, undefined, parent as any, { initialTodos, initialStatus: "active", startTask: false, + taskContext: childTaskContext, }) - // 5) Persist parent delegation metadata BEFORE the child starts writing. + // 6) Persist parent delegation metadata BEFORE the child starts writing. try { const { historyItem } = await this.getTaskWithId(parentTaskId) const childIds = Array.from(new Set([...(historyItem.childIds ?? []), child.taskId])) @@ -3135,10 +3149,10 @@ export class ClineProvider ) } - // 6) Start the child task now that parent metadata is safely persisted. + // 7) Start the child task now that parent metadata is safely persisted. child.start() - // 7) Emit TaskDelegated (provider-level) + // 8) Emit TaskDelegated (provider-level) try { this.emit(RooCodeEventName.TaskDelegated, parentTaskId, child.taskId) } catch { From c1402987456ccc98e198d17fad74d26f3f5c1fd4 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Tue, 12 May 2026 05:29:54 +0000 Subject: [PATCH 05/11] feat: add model-driven permission control for subtasks (Phase 3b) Adds an optional `permissions` parameter to the `new_task` tool, allowing the Orchestrator (or any parent task) to dynamically set permission boundaries for subtasks: - New `TaskPermissions` type with filePatterns, commandPatterns, allowedTools, and deniedTools - Permission merging with most-restrictive-wins semantics for nested subtask delegation - Runtime enforcement in validateToolUse() for all permission types - Full test coverage for merging logic and enforcement Addresses Issue #12330 (Phase 3b) --- .../src/__tests__/task-permissions.spec.ts | 152 ++++++++++++ packages/types/src/index.ts | 1 + packages/types/src/task-permissions.ts | 124 ++++++++++ packages/types/src/task.ts | 4 + .../assistant-message/NativeToolCallParser.ts | 4 + .../presentAssistantMessage.ts | 1 + .../prompts/tools/native-tools/new_task.ts | 5 + src/core/task/Task.ts | 7 + src/core/tools/NewTaskTool.ts | 33 ++- .../taskPermissionsEnforcement.spec.ts | 219 ++++++++++++++++++ src/core/tools/validateToolUse.ts | 83 ++++++- src/core/webview/ClineProvider.ts | 1 + src/shared/tools.ts | 4 +- 13 files changed, 634 insertions(+), 4 deletions(-) create mode 100644 packages/types/src/__tests__/task-permissions.spec.ts create mode 100644 packages/types/src/task-permissions.ts create mode 100644 src/core/tools/__tests__/taskPermissionsEnforcement.spec.ts diff --git a/packages/types/src/__tests__/task-permissions.spec.ts b/packages/types/src/__tests__/task-permissions.spec.ts new file mode 100644 index 00000000000..101c487d5a4 --- /dev/null +++ b/packages/types/src/__tests__/task-permissions.spec.ts @@ -0,0 +1,152 @@ +import { describe, it, expect } from "vitest" +import { mergeTaskPermissions, matchesAnyPattern, taskPermissionsSchema } from "../task-permissions.js" +import type { TaskPermissions } from "../task-permissions.js" + +describe("TaskPermissions", () => { + describe("taskPermissionsSchema", () => { + it("validates a valid permissions object", () => { + const result = taskPermissionsSchema.safeParse({ + filePatterns: ["src/components/.*"], + commandPatterns: ["npm test.*"], + allowedTools: ["read_file", "write_to_file"], + deniedTools: ["execute_command"], + }) + expect(result.success).toBe(true) + }) + + it("validates an empty object", () => { + const result = taskPermissionsSchema.safeParse({}) + expect(result.success).toBe(true) + }) + + it("validates partial permissions", () => { + const result = taskPermissionsSchema.safeParse({ + filePatterns: ["src/.*"], + }) + expect(result.success).toBe(true) + }) + + it("rejects non-string array values", () => { + const result = taskPermissionsSchema.safeParse({ + filePatterns: [123], + }) + expect(result.success).toBe(false) + }) + }) + + describe("mergeTaskPermissions", () => { + it("returns undefined when both are undefined", () => { + expect(mergeTaskPermissions(undefined, undefined)).toBeUndefined() + }) + + it("returns child when parent is undefined", () => { + const child: TaskPermissions = { filePatterns: ["src/.*"] } + expect(mergeTaskPermissions(undefined, child)).toEqual(child) + }) + + it("returns parent when child is undefined", () => { + const parent: TaskPermissions = { filePatterns: ["src/.*"] } + expect(mergeTaskPermissions(parent, undefined)).toEqual(parent) + }) + + it("intersects filePatterns when both defined", () => { + const parent: TaskPermissions = { filePatterns: ["src/.*", "tests/.*"] } + const child: TaskPermissions = { filePatterns: ["src/.*", "docs/.*"] } + const merged = mergeTaskPermissions(parent, child) + expect(merged?.filePatterns).toEqual(["src/.*"]) + }) + + it("intersects commandPatterns when both defined", () => { + const parent: TaskPermissions = { commandPatterns: ["npm test.*", "npm run lint"] } + const child: TaskPermissions = { commandPatterns: ["npm test.*", "npm run build"] } + const merged = mergeTaskPermissions(parent, child) + expect(merged?.commandPatterns).toEqual(["npm test.*"]) + }) + + it("intersects allowedTools when both defined", () => { + const parent: TaskPermissions = { allowedTools: ["read_file", "write_to_file", "search_files"] } + const child: TaskPermissions = { allowedTools: ["read_file", "execute_command"] } + const merged = mergeTaskPermissions(parent, child) + expect(merged?.allowedTools).toEqual(["read_file"]) + }) + + it("unions deniedTools when both defined", () => { + const parent: TaskPermissions = { deniedTools: ["execute_command"] } + const child: TaskPermissions = { deniedTools: ["write_to_file"] } + const merged = mergeTaskPermissions(parent, child) + expect(merged?.deniedTools).toEqual(["execute_command", "write_to_file"]) + }) + + it("deduplicates deniedTools in union", () => { + const parent: TaskPermissions = { deniedTools: ["execute_command", "write_to_file"] } + const child: TaskPermissions = { deniedTools: ["execute_command", "search_files"] } + const merged = mergeTaskPermissions(parent, child) + expect(merged?.deniedTools).toEqual(["execute_command", "write_to_file", "search_files"]) + }) + + it("uses parent filePatterns when child has none", () => { + const parent: TaskPermissions = { filePatterns: ["src/.*"] } + const child: TaskPermissions = { deniedTools: ["execute_command"] } + const merged = mergeTaskPermissions(parent, child) + expect(merged?.filePatterns).toEqual(["src/.*"]) + expect(merged?.deniedTools).toEqual(["execute_command"]) + }) + + it("returns empty array when intersection is empty", () => { + const parent: TaskPermissions = { allowedTools: ["read_file"] } + const child: TaskPermissions = { allowedTools: ["write_to_file"] } + const merged = mergeTaskPermissions(parent, child) + expect(merged?.allowedTools).toEqual([]) + }) + + it("handles complex nested merge scenario with exact string matching", () => { + const grandparent: TaskPermissions = { + filePatterns: ["src/.*"], + commandPatterns: ["npm.*"], + allowedTools: ["read_file", "write_to_file", "search_files"], + deniedTools: ["execute_command"], + } + const parent: TaskPermissions = { + filePatterns: ["src/components/.*"], + allowedTools: ["read_file", "write_to_file"], + } + + // Intersection uses exact string matching, so "src/components/.*" (child) + // is not equal to "src/.*" (parent) -- intersection is empty + const merged1 = mergeTaskPermissions(grandparent, parent) + expect(merged1?.filePatterns).toEqual([]) + // allowedTools intersection: read_file and write_to_file are in both + expect(merged1?.allowedTools).toEqual(["read_file", "write_to_file"]) + // commandPatterns: only grandparent has them, so they pass through + expect(merged1?.commandPatterns).toEqual(["npm.*"]) + // deniedTools: only grandparent has them, so they pass through + expect(merged1?.deniedTools).toEqual(["execute_command"]) + }) + }) + + describe("matchesAnyPattern", () => { + it("matches a simple regex pattern", () => { + expect(matchesAnyPattern("src/components/Button.tsx", ["src/components/.*"])).toBe(true) + }) + + it("does not match when no patterns match", () => { + expect(matchesAnyPattern("tests/unit/test.ts", ["src/components/.*"])).toBe(false) + }) + + it("matches when at least one pattern matches", () => { + expect(matchesAnyPattern("tests/unit/test.ts", ["src/.*", "tests/.*"])).toBe(true) + }) + + it("handles invalid regex gracefully", () => { + expect(matchesAnyPattern("test.ts", ["[invalid"])).toBe(false) + }) + + it("matches command patterns", () => { + expect(matchesAnyPattern("npm test -- --coverage", ["npm test.*"])).toBe(true) + }) + + it("does not match restricted commands", () => { + expect(matchesAnyPattern("rm -rf /", ["npm.*", "yarn.*"])).toBe(false) + }) + }) +}) diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index d147f2d94d5..fb723bc41f6 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -21,6 +21,7 @@ export * from "./model.js" export * from "./provider-settings.js" export * from "./task.js" export * from "./task-context.js" +export * from "./task-permissions.js" export * from "./todo.js" export * from "./skills.js" export * from "./terminal.js" diff --git a/packages/types/src/task-permissions.ts b/packages/types/src/task-permissions.ts new file mode 100644 index 00000000000..96698fe8bf4 --- /dev/null +++ b/packages/types/src/task-permissions.ts @@ -0,0 +1,124 @@ +import { z } from "zod" + +/** + * TaskPermissions defines permission boundaries that a parent task can impose + * on a subtask created via the `new_task` tool. + * + * When nested subtasks are created, permissions are merged using + * "most-restrictive-wins" semantics: a child can never grant itself + * more access than its parent. + */ + +export const taskPermissionsSchema = z.object({ + /** + * Regex patterns for allowed file paths. + * When set, file operations (read/write) are restricted to paths matching + * at least one of these patterns. + */ + filePatterns: z.array(z.string()).optional(), + + /** + * Regex patterns for allowed shell commands. + * When set, command execution is restricted to commands matching + * at least one of these patterns. + */ + commandPatterns: z.array(z.string()).optional(), + + /** + * Explicit tool allowlist. When set, only these tools may be used + * by the subtask (in addition to always-available tools like + * attempt_completion and ask_followup_question). + */ + allowedTools: z.array(z.string()).optional(), + + /** + * Explicit tool blocklist. These tools are denied regardless of + * mode configuration. + */ + deniedTools: z.array(z.string()).optional(), +}) + +export type TaskPermissions = z.infer + +/** + * Merge two TaskPermissions using most-restrictive-wins semantics. + * + * - filePatterns / commandPatterns: if both define patterns, keep only patterns + * present in both (intersection). If only one side defines patterns, use that. + * - allowedTools: intersection of both lists (if both defined). + * - deniedTools: union of both lists (most restrictive). + * + * @returns merged permissions, or undefined if both inputs are undefined. + */ +export function mergeTaskPermissions( + parent: TaskPermissions | undefined, + child: TaskPermissions | undefined, +): TaskPermissions | undefined { + if (!parent && !child) { + return undefined + } + if (!parent) { + return child + } + if (!child) { + return parent + } + + return { + filePatterns: intersectOptionalArrays(parent.filePatterns, child.filePatterns), + commandPatterns: intersectOptionalArrays(parent.commandPatterns, child.commandPatterns), + allowedTools: intersectOptionalArrays(parent.allowedTools, child.allowedTools), + deniedTools: unionOptionalArrays(parent.deniedTools, child.deniedTools), + } +} + +/** + * Check if a value matches at least one pattern in a list of regex patterns. + */ +export function matchesAnyPattern(value: string, patterns: string[]): boolean { + return patterns.some((pattern) => { + try { + return new RegExp(pattern).test(value) + } catch { + // Invalid regex -- treat as non-match + return false + } + }) +} + +/** + * Intersect two optional arrays. If both are defined, return elements present + * in both. If only one is defined, return that one. If neither, return undefined. + */ +function intersectOptionalArrays(a: string[] | undefined, b: string[] | undefined): string[] | undefined { + if (!a && !b) { + return undefined + } + if (!a) { + return b + } + if (!b) { + return a + } + + const setB = new Set(b) + const result = a.filter((item) => setB.has(item)) + return result.length > 0 ? result : [] +} + +/** + * Union two optional arrays, deduplicating entries. + */ +function unionOptionalArrays(a: string[] | undefined, b: string[] | undefined): string[] | undefined { + if (!a && !b) { + return undefined + } + if (!a) { + return b + } + if (!b) { + return a + } + + return [...new Set([...a, ...b])] +} diff --git a/packages/types/src/task.ts b/packages/types/src/task.ts index e664714a3a5..d0fa680f8fc 100644 --- a/packages/types/src/task.ts +++ b/packages/types/src/task.ts @@ -4,6 +4,7 @@ import { RooCodeEventName } from "./events.js" import type { RooCodeSettings } from "./global-settings.js" import type { ClineMessage, QueuedMessage, TokenUsage } from "./message.js" import type { ToolUsage, ToolName } from "./tool.js" +import type { TaskPermissions } from "./task-permissions.js" import type { TodoItem } from "./todo.js" import type { TaskContext } from "./task-context.js" @@ -101,6 +102,9 @@ export interface CreateTaskOptions { * Phase 3a foundation for concurrent task execution. */ taskContext?: TaskContext + /** Permission boundaries for the task, set by the parent via new_task tool. + * When set, restricts what file paths, commands, and tools the task may use. */ + taskPermissions?: TaskPermissions } export enum TaskStatus { diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index 30bf119e4fd..6ff2aef2741 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -551,6 +551,7 @@ export class NativeToolCallParser { if (partialArgs.todos !== undefined) { nativeArgs = { todos: partialArgs.todos, + permissions: partialArgs.permissions, } } break @@ -634,6 +635,7 @@ export class NativeToolCallParser { message: partialArgs.message, todos: partialArgs.todos, task_queue: partialArgs.task_queue, + permissions: partialArgs.permissions, } } break @@ -888,6 +890,7 @@ export class NativeToolCallParser { if (args.todos !== undefined) { nativeArgs = { todos: args.todos, + permissions: args.permissions, } as NativeArgsFor } break @@ -984,6 +987,7 @@ export class NativeToolCallParser { message: args.message, todos: args.todos, task_queue: args.task_queue, + permissions: args.permissions, } as NativeArgsFor } break diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 49ce56a305d..dc0c98c7d11 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -589,6 +589,7 @@ export async function presentAssistantMessage(cline: Task) { block.params, stateExperiments, includedTools, + cline.taskPermissions, ) } catch (error) { cline.consecutiveMistakeCount++ diff --git a/src/core/prompts/tools/native-tools/new_task.ts b/src/core/prompts/tools/native-tools/new_task.ts index 051351a9a80..97a7eb1da25 100644 --- a/src/core/prompts/tools/native-tools/new_task.ts +++ b/src/core/prompts/tools/native-tools/new_task.ts @@ -13,6 +13,7 @@ const MESSAGE_PARAMETER_DESCRIPTION = `Initial user instructions or context for const TODOS_PARAMETER_DESCRIPTION = `Optional initial todo list written as a markdown checklist; required when the workspace mandates todos` const TASK_QUEUE_PARAMETER_DESCRIPTION = `Optional JSON array of additional subtasks to execute sequentially after the first subtask completes. Each element is an object with "mode" (string) and "message" (string). Example: [{"mode":"code","message":"Implement feature X"},{"mode":"debug","message":"Test feature X"}]. When provided, the system automatically transitions between subtasks without returning to the parent, collecting all results. The parent receives aggregated results when the entire queue completes.` +const PERMISSIONS_PARAMETER_DESCRIPTION = `Optional JSON object defining permission boundaries for the subtask. Allows the parent to restrict the subtask's access. Supports: filePatterns (array of regex patterns for allowed file paths), commandPatterns (array of regex patterns for allowed commands), allowedTools (array of tool names the subtask may use), deniedTools (array of tool names the subtask may NOT use). Example: {"filePatterns":["src/components/.*"],"commandPatterns":["npm test.*"],"deniedTools":["execute_command"]}` export default { type: "function", @@ -39,6 +40,10 @@ export default { type: ["string", "null"], description: TASK_QUEUE_PARAMETER_DESCRIPTION, }, + permissions: { + type: ["string", "null"], + description: PERMISSIONS_PARAMETER_DESCRIPTION, + }, }, required: ["mode", "message", "todos"], additionalProperties: false, diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index fe5241188fc..df7acbdcb75 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -39,6 +39,8 @@ import { TaskStatus, TodoItem, getApiProtocol, + type TaskPermissions, + mergeTaskPermissions, getModelId, isRetiredProvider, isIdleAsk, @@ -169,6 +171,7 @@ export class Task extends EventEmitter implements TaskLike { readonly taskId: string readonly rootTaskId?: string readonly parentTaskId?: string + readonly taskPermissions?: TaskPermissions childTaskId?: string pendingNewTaskToolCallId?: string @@ -450,6 +453,7 @@ export class Task extends EventEmitter implements TaskLike { workspacePath, initialStatus, taskContext, + taskPermissions, }: TaskOptions) { super() @@ -476,6 +480,9 @@ export class Task extends EventEmitter implements TaskLike { this.parentTaskId = historyItem ? historyItem.parentTaskId : parentTask?.taskId this.childTaskId = undefined + // Merge task permissions with parent (most-restrictive-wins) + this.taskPermissions = mergeTaskPermissions(parentTask?.taskPermissions, taskPermissions) + this.metadata = { task: historyItem ? historyItem.task : task, images: historyItem ? [] : images, diff --git a/src/core/tools/NewTaskTool.ts b/src/core/tools/NewTaskTool.ts index 636343a489b..df6d0812b9d 100644 --- a/src/core/tools/NewTaskTool.ts +++ b/src/core/tools/NewTaskTool.ts @@ -2,6 +2,7 @@ import * as vscode from "vscode" import { TodoItem } from "@roo-code/types" import type { SubtaskQueueItem } from "@roo-code/types" +import { type TaskPermissions, taskPermissionsSchema, toTaskPermissions } from "@roo-code/types" import { Task } from "../task/Task" import { getModeBySlug } from "../../shared/modes" @@ -17,13 +18,14 @@ interface NewTaskParams { message: string todos?: string task_queue?: string + permissions?: string } export class NewTaskTool extends BaseTool<"new_task"> { readonly name = "new_task" as const async execute(params: NewTaskParams, task: Task, callbacks: ToolCallbacks): Promise { - const { mode, message, todos, task_queue } = params + const { mode, message, todos, task_queue, permissions: permissionsJson } = params const { askApproval, handleError, pushToolResult } = callbacks try { @@ -84,6 +86,33 @@ export class NewTaskTool extends BaseTool<"new_task"> { } } + // Parse and validate permissions if provided + let parsedPermissions: TaskPermissions | undefined + if (permissionsJson) { + try { + const raw = JSON.parse(permissionsJson) + const result = taskPermissionsSchema.safeParse(raw) + if (!result.success) { + task.consecutiveMistakeCount++ + task.recordToolError("new_task") + task.didToolFailInCurrentTurn = true + pushToolResult( + formatResponse.toolError( + `Invalid permissions format: ${result.error.issues.map((i) => i.message).join(", ")}`, + ), + ) + return + } + parsedPermissions = result.data + } catch (error) { + task.consecutiveMistakeCount++ + task.recordToolError("new_task") + task.didToolFailInCurrentTurn = true + pushToolResult(formatResponse.toolError("Invalid permissions: must be a valid JSON string")) + return + } + } + task.consecutiveMistakeCount = 0 // Un-escape one level of backslashes before '@' for hierarchical subtasks @@ -139,6 +168,7 @@ export class NewTaskTool extends BaseTool<"new_task"> { content: message, todos: todoItems, taskQueue: queueItems.length > 0 ? queueItems : undefined, + ...(parsedPermissions ? { permissions: parsedPermissions } : {}), }) const didApprove = await askApproval("tool", toolMessage) @@ -154,6 +184,7 @@ export class NewTaskTool extends BaseTool<"new_task"> { initialTodos: todoItems, mode, subtaskQueue: queueItems.length > 0 ? queueItems : undefined, + permissions: parsedPermissions, }) // Reflect delegation in tool result (no pause/unpause, no wait) diff --git a/src/core/tools/__tests__/taskPermissionsEnforcement.spec.ts b/src/core/tools/__tests__/taskPermissionsEnforcement.spec.ts new file mode 100644 index 00000000000..3faa2e5e876 --- /dev/null +++ b/src/core/tools/__tests__/taskPermissionsEnforcement.spec.ts @@ -0,0 +1,219 @@ +import { describe, it, expect } from "vitest" +import { isToolAllowedForMode, TaskPermissionError } from "../validateToolUse" +import type { TaskPermissions } from "@roo-code/types" +import type { ModeConfig } from "@roo-code/types" + +const codeMode: ModeConfig = { + slug: "code", + name: "Code", + roleDefinition: "You are a coder", + groups: ["read", "edit", "command", "mcp"], +} + +describe("TaskPermissions enforcement in isToolAllowedForMode", () => { + describe("deniedTools", () => { + it("throws TaskPermissionError when tool is in deniedTools", () => { + const permissions: TaskPermissions = { + deniedTools: ["execute_command"], + } + expect(() => + isToolAllowedForMode( + "execute_command", + "code", + [codeMode], + undefined, + undefined, + undefined, + undefined, + permissions, + ), + ).toThrow(TaskPermissionError) + }) + + it("allows tools not in deniedTools", () => { + const permissions: TaskPermissions = { + deniedTools: ["execute_command"], + } + expect( + isToolAllowedForMode( + "read_file", + "code", + [codeMode], + undefined, + undefined, + undefined, + undefined, + permissions, + ), + ).toBe(true) + }) + }) + + describe("allowedTools", () => { + it("throws TaskPermissionError when tool is not in allowedTools", () => { + const permissions: TaskPermissions = { + allowedTools: ["read_file", "search_files"], + } + expect(() => + isToolAllowedForMode( + "write_to_file", + "code", + [codeMode], + undefined, + undefined, + undefined, + undefined, + permissions, + ), + ).toThrow(TaskPermissionError) + }) + + it("allows tools in allowedTools", () => { + const permissions: TaskPermissions = { + allowedTools: ["read_file", "write_to_file"], + } + expect( + isToolAllowedForMode( + "read_file", + "code", + [codeMode], + undefined, + undefined, + undefined, + undefined, + permissions, + ), + ).toBe(true) + }) + + it("always allows ALWAYS_AVAILABLE_TOOLS even when allowedTools is set", () => { + const permissions: TaskPermissions = { + allowedTools: ["read_file"], + } + // attempt_completion and ask_followup_question should always be allowed + expect( + isToolAllowedForMode( + "attempt_completion", + "code", + [codeMode], + undefined, + undefined, + undefined, + undefined, + permissions, + ), + ).toBe(true) + }) + }) + + describe("filePatterns", () => { + it("throws TaskPermissionError when file path doesn't match any pattern", () => { + const permissions: TaskPermissions = { + filePatterns: ["src/components/.*"], + } + expect(() => + isToolAllowedForMode( + "write_to_file", + "code", + [codeMode], + undefined, + { path: "src/utils/helper.ts" }, + undefined, + undefined, + permissions, + ), + ).toThrow(TaskPermissionError) + }) + + it("allows file paths matching a pattern", () => { + const permissions: TaskPermissions = { + filePatterns: ["src/components/.*"], + } + expect( + isToolAllowedForMode( + "write_to_file", + "code", + [codeMode], + undefined, + { path: "src/components/Button.tsx" }, + undefined, + undefined, + permissions, + ), + ).toBe(true) + }) + + it("does not restrict tools without file paths", () => { + const permissions: TaskPermissions = { + filePatterns: ["src/components/.*"], + } + expect( + isToolAllowedForMode( + "search_files", + "code", + [codeMode], + undefined, + { regex: "TODO" }, + undefined, + undefined, + permissions, + ), + ).toBe(true) + }) + }) + + describe("commandPatterns", () => { + it("throws TaskPermissionError when command doesn't match any pattern", () => { + const permissions: TaskPermissions = { + commandPatterns: ["npm test.*", "npm run lint"], + } + expect(() => + isToolAllowedForMode( + "execute_command", + "code", + [codeMode], + undefined, + { command: "rm -rf /" }, + undefined, + undefined, + permissions, + ), + ).toThrow(TaskPermissionError) + }) + + it("allows commands matching a pattern", () => { + const permissions: TaskPermissions = { + commandPatterns: ["npm test.*", "npm run lint"], + } + expect( + isToolAllowedForMode( + "execute_command", + "code", + [codeMode], + undefined, + { command: "npm test -- --coverage" }, + undefined, + undefined, + permissions, + ), + ).toBe(true) + }) + }) + + describe("no permissions", () => { + it("allows all tools when taskPermissions is undefined", () => { + expect( + isToolAllowedForMode( + "execute_command", + "code", + [codeMode], + undefined, + undefined, + undefined, + undefined, + undefined, + ), + ).toBe(true) + }) + }) +}) diff --git a/src/core/tools/validateToolUse.ts b/src/core/tools/validateToolUse.ts index 243a170ed90..010c9371b20 100644 --- a/src/core/tools/validateToolUse.ts +++ b/src/core/tools/validateToolUse.ts @@ -1,5 +1,5 @@ -import type { ToolName, ModeConfig, ExperimentId, GroupOptions, GroupEntry } from "@roo-code/types" -import { toolNames as validToolNames } from "@roo-code/types" +import type { ToolName, ModeConfig, ExperimentId, GroupOptions, GroupEntry, TaskPermissions } from "@roo-code/types" +import { toolNames as validToolNames, matchesAnyPattern } from "@roo-code/types" import { customToolRegistry } from "@roo-code/core" import { type Mode, FileRestrictionError, getModeBySlug, getGroupName } from "../../shared/modes" @@ -37,6 +37,7 @@ export function validateToolUse( toolParams?: Record, experiments?: Record, includedTools?: string[], + taskPermissions?: TaskPermissions, ): void { // First, check if the tool name is actually a valid/known tool // This catches completely invalid tool names like "edit_file" that don't exist @@ -56,6 +57,7 @@ export function validateToolUse( toolParams, experiments, includedTools, + taskPermissions, ) ) { throw new Error(`Tool "${toolName}" is not allowed in ${mode} mode.`) @@ -117,6 +119,19 @@ function doesFileMatchRegex(filePath: string, pattern: string): boolean { } } +/** + * Error thrown when a tool is denied by TaskPermissions. + */ +export class TaskPermissionError extends Error { + constructor( + public readonly toolName: string, + public readonly reason: string, + ) { + super(`Tool "${toolName}" is not allowed: ${reason}`) + this.name = "TaskPermissionError" + } +} + export function isToolAllowedForMode( tool: string, modeSlug: string, @@ -125,11 +140,75 @@ export function isToolAllowedForMode( toolParams?: Record, // All tool parameters experiments?: Record, includedTools?: string[], // Opt-in tools explicitly included (e.g., from modelInfo) + taskPermissions?: TaskPermissions, ): boolean { // Resolve alias to canonical name (e.g., "search_and_replace" → "edit") const resolvedTool = TOOL_ALIASES[tool] ?? tool const resolvedIncludedTools = includedTools?.map((t) => TOOL_ALIASES[t] ?? t) + // Check TaskPermissions first -- these are set by the parent task via new_task + if (taskPermissions) { + // Check deniedTools + if (taskPermissions.deniedTools?.includes(resolvedTool) || taskPermissions.deniedTools?.includes(tool)) { + throw new TaskPermissionError(tool, "This tool is denied by the parent task's permission boundaries.") + } + + // Check allowedTools (if set, only these tools are permitted) + if (taskPermissions.allowedTools) { + const isAllowed = + taskPermissions.allowedTools.includes(resolvedTool) || + taskPermissions.allowedTools.includes(tool) || + // Always allow certain critical tools regardless of allowlist + ALWAYS_AVAILABLE_TOOLS.includes(tool as any) + if (!isAllowed) { + throw new TaskPermissionError(tool, "This tool is not in the parent task's allowed tools list.") + } + } + + // Check filePatterns for file-related operations + if (taskPermissions.filePatterns && taskPermissions.filePatterns.length > 0) { + const filePath = toolParams?.path || toolParams?.file_path + if (filePath && typeof filePath === "string") { + if (!matchesAnyPattern(filePath, taskPermissions.filePatterns)) { + throw new TaskPermissionError( + tool, + `File "${filePath}" is outside the allowed file patterns: ${taskPermissions.filePatterns.join(", ")}`, + ) + } + } + + // Check apply_patch file paths + if (tool === "apply_patch" && typeof toolParams?.patch === "string") { + const patchFilePaths = extractFilePathsFromPatch(toolParams.patch) + for (const patchFilePath of patchFilePaths) { + if (!matchesAnyPattern(patchFilePath, taskPermissions.filePatterns)) { + throw new TaskPermissionError( + tool, + `File "${patchFilePath}" in patch is outside the allowed file patterns: ${taskPermissions.filePatterns.join(", ")}`, + ) + } + } + } + } + + // Check commandPatterns for execute_command + if ( + taskPermissions.commandPatterns && + taskPermissions.commandPatterns.length > 0 && + resolvedTool === "execute_command" + ) { + const command = toolParams?.command + if (command && typeof command === "string") { + if (!matchesAnyPattern(command, taskPermissions.commandPatterns)) { + throw new TaskPermissionError( + tool, + `Command "${command}" is outside the allowed command patterns: ${taskPermissions.commandPatterns.join(", ")}`, + ) + } + } + } + } + // Check tool requirements first — explicit disabling takes priority over everything, // including ALWAYS_AVAILABLE_TOOLS. This ensures disabledTools works consistently // at both the filtering layer and the execution-time validation layer. diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index ea7f1f27d27..1431bf8570d 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -3122,6 +3122,7 @@ export class ClineProvider const child = await this.createTask(message, undefined, parent as any, { initialTodos, initialStatus: "active", + taskPermissions: permissions, startTask: false, taskContext: childTaskContext, }) diff --git a/src/shared/tools.ts b/src/shared/tools.ts index b85639d0ae6..c7569e00d2c 100644 --- a/src/shared/tools.ts +++ b/src/shared/tools.ts @@ -57,6 +57,7 @@ export const toolParamNames = [ "end_line", "todos", "task_queue", + "permissions", // new_task parameter for subtask permission boundaries "prompt", "image", // read_file parameters (native protocol) @@ -103,7 +104,7 @@ export type NativeToolArgs = { edit_file: { file_path: string; old_string: string; new_string: string; expected_replacements?: number } apply_patch: { patch: string } list_files: { path: string; recursive?: boolean } - new_task: { mode: string; message: string; todos?: string; task_queue?: string } + new_task: { mode: string; message: string; todos?: string; task_queue?: string; permissions?: string } ask_followup_question: { question: string follow_up: Array<{ text: string; mode?: string }> @@ -242,6 +243,7 @@ export interface SwitchModeToolUse extends ToolUse<"switch_mode"> { export interface NewTaskToolUse extends ToolUse<"new_task"> { name: "new_task" params: Partial, "mode" | "message" | "todos" | "task_queue">> + params: Partial, "mode" | "message" | "todos" | "permissions">> } export interface RunSlashCommandToolUse extends ToolUse<"run_slash_command"> { From c8e6b1778b805feee6d84d1dd8ab2536e5d3870a Mon Sep 17 00:00:00 2001 From: Roo Code Date: Tue, 12 May 2026 05:51:37 +0000 Subject: [PATCH 06/11] fix: three bugs in task permissions - parser, deniedTools exemption, pattern merging 1. NativeToolCallParser: Remove permissions from update_todo_list cases (was erroneously added to wrong tool case, should only be on new_task) 2. deniedTools: Exempt ALWAYS_AVAILABLE_TOOLS (attempt_completion, etc.) from deniedTools check, matching the existing allowedTools behavior. Prevents parent from trapping subtask by denying completion tools. 3. Pattern merging: Replace broken exact-string intersection with layered enforcement. filePatterns/commandPatterns from parent and child are kept as separate layers (AND between layers, OR within each layer). This correctly handles narrowing: parent ["src/.*"] + child ["src/components/.*"] now allows only files matching BOTH patterns, instead of producing an empty intersection. --- .../src/__tests__/task-permissions.spec.ts | 118 ++++++++++++++---- packages/types/src/task-permissions.ts | 117 ++++++++++++++++- .../assistant-message/NativeToolCallParser.ts | 2 - src/core/tools/NewTaskTool.ts | 5 +- .../taskPermissionsEnforcement.spec.ts | 103 +++++++++++++-- src/core/tools/validateToolUse.ts | 54 ++++++-- 6 files changed, 348 insertions(+), 51 deletions(-) diff --git a/packages/types/src/__tests__/task-permissions.spec.ts b/packages/types/src/__tests__/task-permissions.spec.ts index 101c487d5a4..1fae6741452 100644 --- a/packages/types/src/__tests__/task-permissions.spec.ts +++ b/packages/types/src/__tests__/task-permissions.spec.ts @@ -1,5 +1,11 @@ import { describe, it, expect } from "vitest" -import { mergeTaskPermissions, matchesAnyPattern, taskPermissionsSchema } from "../task-permissions.js" +import { + mergeTaskPermissions, + matchesAnyPattern, + matchesAllPatternLayers, + taskPermissionsSchema, + toTaskPermissions, +} from "../task-permissions.js" import type { TaskPermissions } from "../task-permissions.js" describe("TaskPermissions", () => { @@ -34,6 +40,28 @@ describe("TaskPermissions", () => { }) }) + describe("toTaskPermissions", () => { + it("wraps flat filePatterns into a single layer", () => { + const input = { filePatterns: ["src/.*"] } + const result = toTaskPermissions(input) + expect(result._filePatternLayers).toEqual([["src/.*"]]) + expect(result.filePatterns).toEqual(["src/.*"]) + }) + + it("wraps flat commandPatterns into a single layer", () => { + const input = { commandPatterns: ["npm test.*"] } + const result = toTaskPermissions(input) + expect(result._commandPatternLayers).toEqual([["npm test.*"]]) + }) + + it("leaves layers undefined when patterns are not set", () => { + const input = { allowedTools: ["read_file"] } + const result = toTaskPermissions(input) + expect(result._filePatternLayers).toBeUndefined() + expect(result._commandPatternLayers).toBeUndefined() + }) + }) + describe("mergeTaskPermissions", () => { it("returns undefined when both are undefined", () => { expect(mergeTaskPermissions(undefined, undefined)).toBeUndefined() @@ -49,18 +77,25 @@ describe("TaskPermissions", () => { expect(mergeTaskPermissions(parent, undefined)).toEqual(parent) }) - it("intersects filePatterns when both defined", () => { - const parent: TaskPermissions = { filePatterns: ["src/.*", "tests/.*"] } - const child: TaskPermissions = { filePatterns: ["src/.*", "docs/.*"] } + it("accumulates filePatterns as separate layers when both defined", () => { + const parent = toTaskPermissions({ filePatterns: ["src/.*", "tests/.*"] }) + const child = toTaskPermissions({ filePatterns: ["src/.*", "docs/.*"] }) const merged = mergeTaskPermissions(parent, child) - expect(merged?.filePatterns).toEqual(["src/.*"]) + // Both layers are kept (AND semantics between layers) + expect(merged?._filePatternLayers).toEqual([ + ["src/.*", "tests/.*"], + ["src/.*", "docs/.*"], + ]) }) - it("intersects commandPatterns when both defined", () => { - const parent: TaskPermissions = { commandPatterns: ["npm test.*", "npm run lint"] } - const child: TaskPermissions = { commandPatterns: ["npm test.*", "npm run build"] } + it("accumulates commandPatterns as separate layers when both defined", () => { + const parent = toTaskPermissions({ commandPatterns: ["npm test.*", "npm run lint"] }) + const child = toTaskPermissions({ commandPatterns: ["npm test.*", "npm run build"] }) const merged = mergeTaskPermissions(parent, child) - expect(merged?.commandPatterns).toEqual(["npm test.*"]) + expect(merged?._commandPatternLayers).toEqual([ + ["npm test.*", "npm run lint"], + ["npm test.*", "npm run build"], + ]) }) it("intersects allowedTools when both defined", () => { @@ -85,42 +120,50 @@ describe("TaskPermissions", () => { }) it("uses parent filePatterns when child has none", () => { - const parent: TaskPermissions = { filePatterns: ["src/.*"] } + const parent = toTaskPermissions({ filePatterns: ["src/.*"] }) const child: TaskPermissions = { deniedTools: ["execute_command"] } const merged = mergeTaskPermissions(parent, child) - expect(merged?.filePatterns).toEqual(["src/.*"]) + expect(merged?._filePatternLayers).toEqual([["src/.*"]]) expect(merged?.deniedTools).toEqual(["execute_command"]) }) - it("returns empty array when intersection is empty", () => { + it("returns empty array when allowedTools intersection is empty", () => { const parent: TaskPermissions = { allowedTools: ["read_file"] } const child: TaskPermissions = { allowedTools: ["write_to_file"] } const merged = mergeTaskPermissions(parent, child) expect(merged?.allowedTools).toEqual([]) }) - it("handles complex nested merge scenario with exact string matching", () => { - const grandparent: TaskPermissions = { + it("handles nested delegation where child narrows scope", () => { + const grandparent = toTaskPermissions({ filePatterns: ["src/.*"], commandPatterns: ["npm.*"], allowedTools: ["read_file", "write_to_file", "search_files"], deniedTools: ["execute_command"], - } - const parent: TaskPermissions = { + }) + const parent = toTaskPermissions({ filePatterns: ["src/components/.*"], allowedTools: ["read_file", "write_to_file"], - } + }) - // Intersection uses exact string matching, so "src/components/.*" (child) - // is not equal to "src/.*" (parent) -- intersection is empty - const merged1 = mergeTaskPermissions(grandparent, parent) - expect(merged1?.filePatterns).toEqual([]) + const merged = mergeTaskPermissions(grandparent, parent) + + // Both layers are kept -- runtime enforces AND between them + expect(merged?._filePatternLayers).toEqual([["src/.*"], ["src/components/.*"]]) // allowedTools intersection: read_file and write_to_file are in both - expect(merged1?.allowedTools).toEqual(["read_file", "write_to_file"]) + expect(merged?.allowedTools).toEqual(["read_file", "write_to_file"]) // commandPatterns: only grandparent has them, so they pass through - expect(merged1?.commandPatterns).toEqual(["npm.*"]) + expect(merged?._commandPatternLayers).toEqual([["npm.*"]]) // deniedTools: only grandparent has them, so they pass through - expect(merged1?.deniedTools).toEqual(["execute_command"]) + expect(merged?.deniedTools).toEqual(["execute_command"]) + }) + + it("deduplicates identical pattern layers", () => { + const parent = toTaskPermissions({ filePatterns: ["src/.*"] }) + const child = toTaskPermissions({ filePatterns: ["src/.*"] }) + const merged = mergeTaskPermissions(parent, child) + // Identical layers are deduplicated + expect(merged?._filePatternLayers).toEqual([["src/.*"]]) }) }) @@ -149,4 +192,31 @@ describe("TaskPermissions", () => { expect(matchesAnyPattern("rm -rf /", ["npm.*", "yarn.*"])).toBe(false) }) }) + + describe("matchesAllPatternLayers", () => { + it("returns true when layers is undefined", () => { + expect(matchesAllPatternLayers("anything", undefined)).toBe(true) + }) + + it("returns true when layers is empty", () => { + expect(matchesAllPatternLayers("anything", [])).toBe(true) + }) + + it("returns true when value matches all layers", () => { + const layers = [["src/.*"], ["src/components/.*"]] + expect(matchesAllPatternLayers("src/components/Button.tsx", layers)).toBe(true) + }) + + it("returns false when value fails to match one layer", () => { + const layers = [["src/.*"], ["src/components/.*"]] + // Matches src/.* but not src/components/.* + expect(matchesAllPatternLayers("src/utils/helper.ts", layers)).toBe(false) + }) + + it("handles single layer like matchesAnyPattern", () => { + const layers = [["src/.*", "tests/.*"]] + expect(matchesAllPatternLayers("tests/unit/test.ts", layers)).toBe(true) + expect(matchesAllPatternLayers("docs/readme.md", layers)).toBe(false) + }) + }) }) diff --git a/packages/types/src/task-permissions.ts b/packages/types/src/task-permissions.ts index 96698fe8bf4..a7e1daebc63 100644 --- a/packages/types/src/task-permissions.ts +++ b/packages/types/src/task-permissions.ts @@ -38,13 +38,46 @@ export const taskPermissionsSchema = z.object({ deniedTools: z.array(z.string()).optional(), }) -export type TaskPermissions = z.infer +/** The shape accepted as input from the model via the new_task tool. */ +export type TaskPermissionsInput = z.infer + +/** + * Internal representation of task permissions. Extends the input shape with + * layered pattern fields that accumulate across nested delegation so that + * each ancestor's constraints are enforced independently (AND semantics + * between layers, OR semantics within a layer). + */ +export interface TaskPermissions extends TaskPermissionsInput { + /** + * Accumulated file-pattern layers from ancestor tasks. + * Each inner array is an OR-group; all layers must match (AND between layers). + * Populated only by `mergeTaskPermissions` -- never set from model input. + */ + _filePatternLayers?: string[][] + /** + * Accumulated command-pattern layers from ancestor tasks. + * Same semantics as `_filePatternLayers`. + */ + _commandPatternLayers?: string[][] +} + +/** + * Convert a validated input object (flat arrays) into the internal + * `TaskPermissions` representation, wrapping patterns into single layers. + */ +export function toTaskPermissions(input: TaskPermissionsInput): TaskPermissions { + return { + ...input, + _filePatternLayers: input.filePatterns ? [input.filePatterns] : undefined, + _commandPatternLayers: input.commandPatterns ? [input.commandPatterns] : undefined, + } +} /** * Merge two TaskPermissions using most-restrictive-wins semantics. * - * - filePatterns / commandPatterns: if both define patterns, keep only patterns - * present in both (intersection). If only one side defines patterns, use that. + * - filePatterns / commandPatterns: accumulated as independent layers so that + * a value must match at least one pattern from EACH ancestor's layer. * - allowedTools: intersection of both lists (if both defined). * - deniedTools: union of both lists (most restrictive). * @@ -64,14 +97,77 @@ export function mergeTaskPermissions( return parent } + // Collect pattern layers from both sides. Each side may already carry + // accumulated layers from earlier merges (_*PatternLayers) as well as + // its own top-level patterns (filePatterns / commandPatterns). + const filePatternLayers = collectPatternLayers( + parent._filePatternLayers, + parent.filePatterns, + child._filePatternLayers, + child.filePatterns, + ) + + const commandPatternLayers = collectPatternLayers( + parent._commandPatternLayers, + parent.commandPatterns, + child._commandPatternLayers, + child.commandPatterns, + ) + return { - filePatterns: intersectOptionalArrays(parent.filePatterns, child.filePatterns), - commandPatterns: intersectOptionalArrays(parent.commandPatterns, child.commandPatterns), + // The top-level field stores the child's own patterns (used for display / + // serialization); runtime enforcement uses the layers. + filePatterns: child.filePatterns ?? parent.filePatterns, + commandPatterns: child.commandPatterns ?? parent.commandPatterns, + _filePatternLayers: filePatternLayers.length > 0 ? filePatternLayers : undefined, + _commandPatternLayers: commandPatternLayers.length > 0 ? commandPatternLayers : undefined, allowedTools: intersectOptionalArrays(parent.allowedTools, child.allowedTools), deniedTools: unionOptionalArrays(parent.deniedTools, child.deniedTools), } } +/** + * Collect pattern layers from parent and child, deduplicating identical layers. + */ +function collectPatternLayers( + parentLayers: string[][] | undefined, + parentPatterns: string[] | undefined, + childLayers: string[][] | undefined, + childPatterns: string[] | undefined, +): string[][] { + const layers: string[][] = [] + const seen = new Set() + + const addLayer = (layer: string[]) => { + if (layer.length === 0) return + const key = JSON.stringify(layer) + if (!seen.has(key)) { + seen.add(key) + layers.push(layer) + } + } + + // Add accumulated parent layers + if (parentLayers) { + for (const layer of parentLayers) { + addLayer(layer) + } + } else if (parentPatterns && parentPatterns.length > 0) { + addLayer(parentPatterns) + } + + // Add accumulated child layers + if (childLayers) { + for (const layer of childLayers) { + addLayer(layer) + } + } else if (childPatterns && childPatterns.length > 0) { + addLayer(childPatterns) + } + + return layers +} + /** * Check if a value matches at least one pattern in a list of regex patterns. */ @@ -86,6 +182,17 @@ export function matchesAnyPattern(value: string, patterns: string[]): boolean { }) } +/** + * Check if a value matches ALL pattern layers (AND between layers, OR within each layer). + * Returns true if there are no layers. + */ +export function matchesAllPatternLayers(value: string, layers: string[][] | undefined): boolean { + if (!layers || layers.length === 0) { + return true + } + return layers.every((layer) => matchesAnyPattern(value, layer)) +} + /** * Intersect two optional arrays. If both are defined, return elements present * in both. If only one is defined, return that one. If neither, return undefined. diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index 6ff2aef2741..3c9f1569e5c 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -551,7 +551,6 @@ export class NativeToolCallParser { if (partialArgs.todos !== undefined) { nativeArgs = { todos: partialArgs.todos, - permissions: partialArgs.permissions, } } break @@ -890,7 +889,6 @@ export class NativeToolCallParser { if (args.todos !== undefined) { nativeArgs = { todos: args.todos, - permissions: args.permissions, } as NativeArgsFor } break diff --git a/src/core/tools/NewTaskTool.ts b/src/core/tools/NewTaskTool.ts index df6d0812b9d..af0bfe5b680 100644 --- a/src/core/tools/NewTaskTool.ts +++ b/src/core/tools/NewTaskTool.ts @@ -1,7 +1,10 @@ import * as vscode from "vscode" import { TodoItem } from "@roo-code/types" +<<<<<<< HEAD import type { SubtaskQueueItem } from "@roo-code/types" +======= +>>>>>>> 6c51a5d52 (fix: three bugs in task permissions - parser, deniedTools exemption, pattern merging) import { type TaskPermissions, taskPermissionsSchema, toTaskPermissions } from "@roo-code/types" import { Task } from "../task/Task" @@ -103,7 +106,7 @@ export class NewTaskTool extends BaseTool<"new_task"> { ) return } - parsedPermissions = result.data + parsedPermissions = toTaskPermissions(result.data) } catch (error) { task.consecutiveMistakeCount++ task.recordToolError("new_task") diff --git a/src/core/tools/__tests__/taskPermissionsEnforcement.spec.ts b/src/core/tools/__tests__/taskPermissionsEnforcement.spec.ts index 3faa2e5e876..f9154e1b214 100644 --- a/src/core/tools/__tests__/taskPermissionsEnforcement.spec.ts +++ b/src/core/tools/__tests__/taskPermissionsEnforcement.spec.ts @@ -1,6 +1,7 @@ import { describe, it, expect } from "vitest" import { isToolAllowedForMode, TaskPermissionError } from "../validateToolUse" import type { TaskPermissions } from "@roo-code/types" +import { toTaskPermissions, mergeTaskPermissions } from "@roo-code/types" import type { ModeConfig } from "@roo-code/types" const codeMode: ModeConfig = { @@ -47,6 +48,38 @@ describe("TaskPermissions enforcement in isToolAllowedForMode", () => { ), ).toBe(true) }) + + it("never denies ALWAYS_AVAILABLE_TOOLS even when in deniedTools", () => { + const permissions: TaskPermissions = { + deniedTools: ["attempt_completion", "ask_followup_question"], + } + // attempt_completion should always be allowed + expect( + isToolAllowedForMode( + "attempt_completion", + "code", + [codeMode], + undefined, + undefined, + undefined, + undefined, + permissions, + ), + ).toBe(true) + // ask_followup_question should always be allowed + expect( + isToolAllowedForMode( + "ask_followup_question", + "code", + [codeMode], + undefined, + undefined, + undefined, + undefined, + permissions, + ), + ).toBe(true) + }) }) describe("allowedTools", () => { @@ -108,9 +141,9 @@ describe("TaskPermissions enforcement in isToolAllowedForMode", () => { describe("filePatterns", () => { it("throws TaskPermissionError when file path doesn't match any pattern", () => { - const permissions: TaskPermissions = { + const permissions = toTaskPermissions({ filePatterns: ["src/components/.*"], - } + }) expect(() => isToolAllowedForMode( "write_to_file", @@ -126,9 +159,9 @@ describe("TaskPermissions enforcement in isToolAllowedForMode", () => { }) it("allows file paths matching a pattern", () => { - const permissions: TaskPermissions = { + const permissions = toTaskPermissions({ filePatterns: ["src/components/.*"], - } + }) expect( isToolAllowedForMode( "write_to_file", @@ -144,9 +177,9 @@ describe("TaskPermissions enforcement in isToolAllowedForMode", () => { }) it("does not restrict tools without file paths", () => { - const permissions: TaskPermissions = { + const permissions = toTaskPermissions({ filePatterns: ["src/components/.*"], - } + }) expect( isToolAllowedForMode( "search_files", @@ -162,11 +195,61 @@ describe("TaskPermissions enforcement in isToolAllowedForMode", () => { }) }) + describe("filePatterns layered enforcement", () => { + it("enforces all pattern layers (AND between layers)", () => { + const parent = toTaskPermissions({ filePatterns: ["src/.*"] }) + const child = toTaskPermissions({ filePatterns: ["src/components/.*"] }) + const merged = mergeTaskPermissions(parent, child)! + + // src/components/Button.tsx matches both layers + expect( + isToolAllowedForMode( + "write_to_file", + "code", + [codeMode], + undefined, + { path: "src/components/Button.tsx" }, + undefined, + undefined, + merged, + ), + ).toBe(true) + + // src/utils/helper.ts matches parent layer but not child layer + expect(() => + isToolAllowedForMode( + "write_to_file", + "code", + [codeMode], + undefined, + { path: "src/utils/helper.ts" }, + undefined, + undefined, + merged, + ), + ).toThrow(TaskPermissionError) + + // tests/test.ts matches neither layer + expect(() => + isToolAllowedForMode( + "write_to_file", + "code", + [codeMode], + undefined, + { path: "tests/test.ts" }, + undefined, + undefined, + merged, + ), + ).toThrow(TaskPermissionError) + }) + }) + describe("commandPatterns", () => { it("throws TaskPermissionError when command doesn't match any pattern", () => { - const permissions: TaskPermissions = { + const permissions = toTaskPermissions({ commandPatterns: ["npm test.*", "npm run lint"], - } + }) expect(() => isToolAllowedForMode( "execute_command", @@ -182,9 +265,9 @@ describe("TaskPermissions enforcement in isToolAllowedForMode", () => { }) it("allows commands matching a pattern", () => { - const permissions: TaskPermissions = { + const permissions = toTaskPermissions({ commandPatterns: ["npm test.*", "npm run lint"], - } + }) expect( isToolAllowedForMode( "execute_command", diff --git a/src/core/tools/validateToolUse.ts b/src/core/tools/validateToolUse.ts index 010c9371b20..d8459c9d01f 100644 --- a/src/core/tools/validateToolUse.ts +++ b/src/core/tools/validateToolUse.ts @@ -1,5 +1,5 @@ import type { ToolName, ModeConfig, ExperimentId, GroupOptions, GroupEntry, TaskPermissions } from "@roo-code/types" -import { toolNames as validToolNames, matchesAnyPattern } from "@roo-code/types" +import { toolNames as validToolNames, matchesAnyPattern, matchesAllPatternLayers } from "@roo-code/types" import { customToolRegistry } from "@roo-code/core" import { type Mode, FileRestrictionError, getModeBySlug, getGroupName } from "../../shared/modes" @@ -148,8 +148,13 @@ export function isToolAllowedForMode( // Check TaskPermissions first -- these are set by the parent task via new_task if (taskPermissions) { - // Check deniedTools - if (taskPermissions.deniedTools?.includes(resolvedTool) || taskPermissions.deniedTools?.includes(tool)) { + const isAlwaysAvailable = ALWAYS_AVAILABLE_TOOLS.includes(tool as any) + + // Check deniedTools (but never deny always-available tools like attempt_completion) + if ( + !isAlwaysAvailable && + (taskPermissions.deniedTools?.includes(resolvedTool) || taskPermissions.deniedTools?.includes(tool)) + ) { throw new TaskPermissionError(tool, "This tool is denied by the parent task's permission boundaries.") } @@ -159,14 +164,37 @@ export function isToolAllowedForMode( taskPermissions.allowedTools.includes(resolvedTool) || taskPermissions.allowedTools.includes(tool) || // Always allow certain critical tools regardless of allowlist - ALWAYS_AVAILABLE_TOOLS.includes(tool as any) + isAlwaysAvailable if (!isAllowed) { throw new TaskPermissionError(tool, "This tool is not in the parent task's allowed tools list.") } } - // Check filePatterns for file-related operations - if (taskPermissions.filePatterns && taskPermissions.filePatterns.length > 0) { + // Check filePatterns using layered enforcement (AND between layers, OR within each layer). + // Falls back to flat filePatterns if no layers are present. + const filePatternLayers = taskPermissions._filePatternLayers + if (filePatternLayers && filePatternLayers.length > 0) { + const filePath = toolParams?.path || toolParams?.file_path + if (filePath && typeof filePath === "string") { + if (!matchesAllPatternLayers(filePath, filePatternLayers)) { + throw new TaskPermissionError(tool, `File "${filePath}" is outside the allowed file patterns.`) + } + } + + // Check apply_patch file paths + if (tool === "apply_patch" && typeof toolParams?.patch === "string") { + const patchFilePaths = extractFilePathsFromPatch(toolParams.patch) + for (const patchFilePath of patchFilePaths) { + if (!matchesAllPatternLayers(patchFilePath, filePatternLayers)) { + throw new TaskPermissionError( + tool, + `File "${patchFilePath}" in patch is outside the allowed file patterns.`, + ) + } + } + } + } else if (taskPermissions.filePatterns && taskPermissions.filePatterns.length > 0) { + // Fallback for non-merged permissions (single layer) const filePath = toolParams?.path || toolParams?.file_path if (filePath && typeof filePath === "string") { if (!matchesAnyPattern(filePath, taskPermissions.filePatterns)) { @@ -177,7 +205,6 @@ export function isToolAllowedForMode( } } - // Check apply_patch file paths if (tool === "apply_patch" && typeof toolParams?.patch === "string") { const patchFilePaths = extractFilePathsFromPatch(toolParams.patch) for (const patchFilePath of patchFilePaths) { @@ -191,12 +218,21 @@ export function isToolAllowedForMode( } } - // Check commandPatterns for execute_command - if ( + // Check commandPatterns using layered enforcement + const commandPatternLayers = taskPermissions._commandPatternLayers + if (commandPatternLayers && commandPatternLayers.length > 0 && resolvedTool === "execute_command") { + const command = toolParams?.command + if (command && typeof command === "string") { + if (!matchesAllPatternLayers(command, commandPatternLayers)) { + throw new TaskPermissionError(tool, `Command "${command}" is outside the allowed command patterns.`) + } + } + } else if ( taskPermissions.commandPatterns && taskPermissions.commandPatterns.length > 0 && resolvedTool === "execute_command" ) { + // Fallback for non-merged permissions (single layer) const command = toolParams?.command if (command && typeof command === "string") { if (!matchesAnyPattern(command, taskPermissions.commandPatterns)) { From 311a2bdfb4f404a760f1ec0467689fb5ddc384d1 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Tue, 12 May 2026 06:04:41 +0000 Subject: [PATCH 07/11] fix: harden task permissions - anchor regex patterns, validate at schema level, simplify validation code 1. Anchor regex patterns in matchesAnyPattern with ^(?:...)$ wrapping so patterns like "src/.*" require full-path matching instead of substring matching. Prevents "evil/src/foo" from matching a "src/.*" permission. 2. Add regex validation at schema level (regexString refinement) so invalid patterns are rejected at parse time rather than silently failing at runtime. 3. Simplify duplicate file/command pattern validation in validateToolUse by unifying layered and flat code paths into a single branch that falls back to wrapping flat patterns as a single layer. 4. Remove unused matchesAnyPattern import from validateToolUse.ts. 5. Add tests for anchoring behavior, pre-anchored patterns, and invalid regex rejection at schema level. --- .../src/__tests__/task-permissions.spec.ts | 27 +++++++++ packages/types/src/task-permissions.ts | 27 +++++++-- src/core/tools/validateToolUse.ts | 56 ++++--------------- 3 files changed, 61 insertions(+), 49 deletions(-) diff --git a/packages/types/src/__tests__/task-permissions.spec.ts b/packages/types/src/__tests__/task-permissions.spec.ts index 1fae6741452..cea98cff38e 100644 --- a/packages/types/src/__tests__/task-permissions.spec.ts +++ b/packages/types/src/__tests__/task-permissions.spec.ts @@ -38,6 +38,20 @@ describe("TaskPermissions", () => { }) expect(result.success).toBe(false) }) + + it("rejects invalid regex patterns in filePatterns", () => { + const result = taskPermissionsSchema.safeParse({ + filePatterns: ["[invalid"], + }) + expect(result.success).toBe(false) + }) + + it("rejects invalid regex patterns in commandPatterns", () => { + const result = taskPermissionsSchema.safeParse({ + commandPatterns: ["(unclosed"], + }) + expect(result.success).toBe(false) + }) }) describe("toTaskPermissions", () => { @@ -191,6 +205,19 @@ describe("TaskPermissions", () => { it("does not match restricted commands", () => { expect(matchesAnyPattern("rm -rf /", ["npm.*", "yarn.*"])).toBe(false) }) + + it("anchors patterns so substrings do not match", () => { + // "src/.*" should NOT match a path that merely contains "src/" as a substring + expect(matchesAnyPattern("evil/src/components/foo.ts", ["src/.*"])).toBe(false) + // But should still match paths that start with src/ + expect(matchesAnyPattern("src/components/foo.ts", ["src/.*"])).toBe(true) + }) + + it("respects pre-anchored patterns (starting with ^)", () => { + // A pattern already starting with ^ should not be double-wrapped + expect(matchesAnyPattern("src/foo.ts", ["^src/.*$"])).toBe(true) + expect(matchesAnyPattern("evil/src/foo.ts", ["^src/.*$"])).toBe(false) + }) }) describe("matchesAllPatternLayers", () => { diff --git a/packages/types/src/task-permissions.ts b/packages/types/src/task-permissions.ts index a7e1daebc63..61f5001ffe9 100644 --- a/packages/types/src/task-permissions.ts +++ b/packages/types/src/task-permissions.ts @@ -9,20 +9,34 @@ import { z } from "zod" * more access than its parent. */ +/** Zod refinement that rejects strings which are not valid regular expressions. */ +const regexString = z.string().refine( + (val) => { + try { + new RegExp(val) + return true + } catch { + return false + } + }, + { message: "Invalid regular expression" }, +) + export const taskPermissionsSchema = z.object({ /** * Regex patterns for allowed file paths. * When set, file operations (read/write) are restricted to paths matching - * at least one of these patterns. + * at least one of these patterns. Patterns are automatically anchored + * (wrapped in `^(?:...)$`) at runtime so they match the full path. */ - filePatterns: z.array(z.string()).optional(), + filePatterns: z.array(regexString).optional(), /** * Regex patterns for allowed shell commands. * When set, command execution is restricted to commands matching - * at least one of these patterns. + * at least one of these patterns. Patterns are automatically anchored. */ - commandPatterns: z.array(z.string()).optional(), + commandPatterns: z.array(regexString).optional(), /** * Explicit tool allowlist. When set, only these tools may be used @@ -174,7 +188,10 @@ function collectPatternLayers( export function matchesAnyPattern(value: string, patterns: string[]): boolean { return patterns.some((pattern) => { try { - return new RegExp(pattern).test(value) + // Anchor patterns so they must match the entire value, not a substring. + // This prevents "src/.*" from matching "evil/src/foo". + const anchored = pattern.startsWith("^") ? pattern : `^(?:${pattern})$` + return new RegExp(anchored).test(value) } catch { // Invalid regex -- treat as non-match return false diff --git a/src/core/tools/validateToolUse.ts b/src/core/tools/validateToolUse.ts index d8459c9d01f..6dd9597f0a6 100644 --- a/src/core/tools/validateToolUse.ts +++ b/src/core/tools/validateToolUse.ts @@ -1,5 +1,5 @@ import type { ToolName, ModeConfig, ExperimentId, GroupOptions, GroupEntry, TaskPermissions } from "@roo-code/types" -import { toolNames as validToolNames, matchesAnyPattern, matchesAllPatternLayers } from "@roo-code/types" +import { toolNames as validToolNames, matchesAllPatternLayers } from "@roo-code/types" import { customToolRegistry } from "@roo-code/core" import { type Mode, FileRestrictionError, getModeBySlug, getGroupName } from "../../shared/modes" @@ -170,9 +170,12 @@ export function isToolAllowedForMode( } } - // Check filePatterns using layered enforcement (AND between layers, OR within each layer). - // Falls back to flat filePatterns if no layers are present. - const filePatternLayers = taskPermissions._filePatternLayers + // Check filePatterns -- use layered enforcement when available (AND between + // layers, OR within each layer), fall back to flat filePatterns as a single layer. + const filePatternLayers = + taskPermissions._filePatternLayers ?? + (taskPermissions.filePatterns?.length ? [taskPermissions.filePatterns] : undefined) + if (filePatternLayers && filePatternLayers.length > 0) { const filePath = toolParams?.path || toolParams?.file_path if (filePath && typeof filePath === "string") { @@ -193,33 +196,13 @@ export function isToolAllowedForMode( } } } - } else if (taskPermissions.filePatterns && taskPermissions.filePatterns.length > 0) { - // Fallback for non-merged permissions (single layer) - const filePath = toolParams?.path || toolParams?.file_path - if (filePath && typeof filePath === "string") { - if (!matchesAnyPattern(filePath, taskPermissions.filePatterns)) { - throw new TaskPermissionError( - tool, - `File "${filePath}" is outside the allowed file patterns: ${taskPermissions.filePatterns.join(", ")}`, - ) - } - } - - if (tool === "apply_patch" && typeof toolParams?.patch === "string") { - const patchFilePaths = extractFilePathsFromPatch(toolParams.patch) - for (const patchFilePath of patchFilePaths) { - if (!matchesAnyPattern(patchFilePath, taskPermissions.filePatterns)) { - throw new TaskPermissionError( - tool, - `File "${patchFilePath}" in patch is outside the allowed file patterns: ${taskPermissions.filePatterns.join(", ")}`, - ) - } - } - } } - // Check commandPatterns using layered enforcement - const commandPatternLayers = taskPermissions._commandPatternLayers + // Check commandPatterns -- same layered approach as filePatterns. + const commandPatternLayers = + taskPermissions._commandPatternLayers ?? + (taskPermissions.commandPatterns?.length ? [taskPermissions.commandPatterns] : undefined) + if (commandPatternLayers && commandPatternLayers.length > 0 && resolvedTool === "execute_command") { const command = toolParams?.command if (command && typeof command === "string") { @@ -227,21 +210,6 @@ export function isToolAllowedForMode( throw new TaskPermissionError(tool, `Command "${command}" is outside the allowed command patterns.`) } } - } else if ( - taskPermissions.commandPatterns && - taskPermissions.commandPatterns.length > 0 && - resolvedTool === "execute_command" - ) { - // Fallback for non-merged permissions (single layer) - const command = toolParams?.command - if (command && typeof command === "string") { - if (!matchesAnyPattern(command, taskPermissions.commandPatterns)) { - throw new TaskPermissionError( - tool, - `Command "${command}" is outside the allowed command patterns: ${taskPermissions.commandPatterns.join(", ")}`, - ) - } - } } } From caf552194f8b33bc3ca8f9f2d0b7dbd5aceaa7ba Mon Sep 17 00:00:00 2001 From: Roo Code Date: Tue, 12 May 2026 06:17:03 +0000 Subject: [PATCH 08/11] fix: persist permissions in HistoryItem and add ReDoS mitigation 1. Persist taskPermissions in HistoryItem so permissions survive task restarts. Added taskPermissions field to historyItemSchema, included it in taskMetadata output, and restored it in the Task constructor when loading from history. 2. Add ReDoS mitigation for model-provided regex patterns: - isSafeRegex() heuristic rejects nested quantifiers like (a+)+ and overlapping alternations in repeated groups like (a|a)+ - Max pattern length capped at 200 characters - Both checks enforced at schema validation time via Zod refinements - 11 new tests covering ReDoS detection and persistence round-trips --- .../src/__tests__/task-permissions.spec.ts | 76 +++++++++++++++++++ packages/types/src/history.ts | 3 + packages/types/src/task-permissions.ts | 68 ++++++++++++++--- src/core/task-persistence/taskMetadata.ts | 6 +- src/core/task/Task.ts | 27 ++++++- 5 files changed, 165 insertions(+), 15 deletions(-) diff --git a/packages/types/src/__tests__/task-permissions.spec.ts b/packages/types/src/__tests__/task-permissions.spec.ts index cea98cff38e..48990c98752 100644 --- a/packages/types/src/__tests__/task-permissions.spec.ts +++ b/packages/types/src/__tests__/task-permissions.spec.ts @@ -5,6 +5,7 @@ import { matchesAllPatternLayers, taskPermissionsSchema, toTaskPermissions, + isSafeRegex, } from "../task-permissions.js" import type { TaskPermissions } from "../task-permissions.js" @@ -246,4 +247,79 @@ describe("TaskPermissions", () => { expect(matchesAllPatternLayers("docs/readme.md", layers)).toBe(false) }) }) + + describe("isSafeRegex", () => { + it("accepts simple file path patterns", () => { + expect(isSafeRegex("src/.*")).toBe(true) + expect(isSafeRegex("src/components/.*\\.tsx")).toBe(true) + expect(isSafeRegex("npm test.*")).toBe(true) + }) + + it("rejects nested quantifiers (classic ReDoS)", () => { + expect(isSafeRegex("(a+)+")).toBe(false) + expect(isSafeRegex("(a*)+")).toBe(false) + expect(isSafeRegex("(a+)*")).toBe(false) + expect(isSafeRegex("(a+){2,}")).toBe(false) + }) + + it("rejects overlapping alternations in repeated groups", () => { + expect(isSafeRegex("(a|a)+")).toBe(false) + expect(isSafeRegex("(.|a)*")).toBe(false) + }) + + it("rejects patterns exceeding maximum length", () => { + const longPattern = "a".repeat(201) + expect(isSafeRegex(longPattern)).toBe(false) + }) + + it("accepts patterns at maximum length", () => { + const maxPattern = "a".repeat(200) + expect(isSafeRegex(maxPattern)).toBe(true) + }) + }) + + describe("schema ReDoS rejection", () => { + it("rejects ReDoS-vulnerable patterns in filePatterns", () => { + const result = taskPermissionsSchema.safeParse({ + filePatterns: ["(a+)+"], + }) + expect(result.success).toBe(false) + }) + + it("rejects ReDoS-vulnerable patterns in commandPatterns", () => { + const result = taskPermissionsSchema.safeParse({ + commandPatterns: ["(cmd|cmd)*"], + }) + expect(result.success).toBe(false) + }) + + it("rejects overly long patterns at schema level", () => { + const result = taskPermissionsSchema.safeParse({ + filePatterns: ["a".repeat(201)], + }) + expect(result.success).toBe(false) + }) + }) + + describe("persistence round-trip", () => { + it("taskPermissionsSchema can parse persisted permissions (without internal fields)", () => { + // Simulate what gets persisted: only the input-level fields + const persisted = { + filePatterns: ["src/.*"], + commandPatterns: ["npm test.*"], + allowedTools: ["read_file"], + deniedTools: ["execute_command"], + } + const result = taskPermissionsSchema.safeParse(persisted) + expect(result.success).toBe(true) + if (result.success) { + // Can be converted back to internal representation + const restored = toTaskPermissions(result.data) + expect(restored._filePatternLayers).toEqual([["src/.*"]]) + expect(restored._commandPatternLayers).toEqual([["npm test.*"]]) + expect(restored.allowedTools).toEqual(["read_file"]) + expect(restored.deniedTools).toEqual(["execute_command"]) + } + }) + }) }) diff --git a/packages/types/src/history.ts b/packages/types/src/history.ts index 2b0a681f48d..74fef5c53e3 100644 --- a/packages/types/src/history.ts +++ b/packages/types/src/history.ts @@ -1,5 +1,7 @@ import { z } from "zod" +import { taskPermissionsSchema } from "./task-permissions.js" + /** * SubtaskQueueItem — a single queued subtask definition for sequential fan-out. * Used by the orchestrator to define a pipeline of subtasks that execute one after another. @@ -52,6 +54,7 @@ export const historyItemSchema = z.object({ subtaskQueue: z.array(subtaskQueueItemSchema).optional(), // Remaining subtasks to execute subtaskQueueIndex: z.number().optional(), // Current position in the original queue (0-based) subtaskResults: z.array(subtaskResultSchema).optional(), // Results from completed queue subtasks + taskPermissions: taskPermissionsSchema.optional(), // Permission boundaries set by parent task }) export type HistoryItem = z.infer diff --git a/packages/types/src/task-permissions.ts b/packages/types/src/task-permissions.ts index 61f5001ffe9..938b321ec77 100644 --- a/packages/types/src/task-permissions.ts +++ b/packages/types/src/task-permissions.ts @@ -9,18 +9,62 @@ import { z } from "zod" * more access than its parent. */ -/** Zod refinement that rejects strings which are not valid regular expressions. */ -const regexString = z.string().refine( - (val) => { - try { - new RegExp(val) - return true - } catch { - return false - } - }, - { message: "Invalid regular expression" }, -) +/** Maximum allowed length for a regex pattern to limit complexity. */ +const MAX_REGEX_PATTERN_LENGTH = 200 + +/** + * Heuristic check for ReDoS-vulnerable patterns. + * Detects common dangerous constructs like nested quantifiers: + * (a+)+, (a*)+, (a+)*, (a*){2,}, etc. + * These can cause catastrophic backtracking on crafted input. + */ +export function isSafeRegex(pattern: string): boolean { + if (pattern.length > MAX_REGEX_PATTERN_LENGTH) { + return false + } + + // Detect nested quantifiers: a group with a quantifier inside, followed by an outer quantifier. + // Examples: (a+)+, (a+)*, (a*){2,}, (?:a+)+ + // This regex looks for: group containing a quantifier, followed by another quantifier + const nestedQuantifierPattern = /\([^)]*[+*][^)]*\)[+*{]/ + if (nestedQuantifierPattern.test(pattern)) { + return false + } + + // Detect overlapping alternations inside repeated groups: (a|a)+, (.|a)+ + // where both alternatives can match the same input + const overlappingAlternationInGroup = /\([^)]*\|[^)]*\)[+*{]/ + if (overlappingAlternationInGroup.test(pattern)) { + return false + } + + return true +} + +/** + * Zod refinement that rejects strings which are not valid regular expressions, + * and also rejects patterns that are vulnerable to ReDoS (catastrophic backtracking). + */ +const regexString = z + .string() + .max(MAX_REGEX_PATTERN_LENGTH, { + message: `Regex pattern must be at most ${MAX_REGEX_PATTERN_LENGTH} characters`, + }) + .refine( + (val) => { + try { + new RegExp(val) + return true + } catch { + return false + } + }, + { message: "Invalid regular expression" }, + ) + .refine((val) => isSafeRegex(val), { + message: + "Regex pattern rejected: potentially vulnerable to ReDoS (catastrophic backtracking). Avoid nested quantifiers like (a+)+ or overlapping alternations in repeated groups.", + }) export const taskPermissionsSchema = z.object({ /** diff --git a/src/core/task-persistence/taskMetadata.ts b/src/core/task-persistence/taskMetadata.ts index 4b771269713..83dee5fb478 100644 --- a/src/core/task-persistence/taskMetadata.ts +++ b/src/core/task-persistence/taskMetadata.ts @@ -1,7 +1,7 @@ import NodeCache from "node-cache" import getFolderSize from "get-folder-size" -import type { ClineMessage, HistoryItem } from "@roo-code/types" +import type { ClineMessage, HistoryItem, TaskPermissionsInput } from "@roo-code/types" import { combineApiRequests } from "../../shared/combineApiRequests" import { combineCommandSequences } from "../../shared/combineCommandSequences" @@ -25,6 +25,8 @@ export type TaskMetadataOptions = { apiConfigName?: string /** Initial status for the task (e.g., "active" for child tasks) */ initialStatus?: "active" | "delegated" | "completed" + /** Permission boundaries for the task, set by the parent via new_task tool */ + taskPermissions?: TaskPermissionsInput } export async function taskMetadata({ @@ -38,6 +40,7 @@ export async function taskMetadata({ mode, apiConfigName, initialStatus, + taskPermissions, }: TaskMetadataOptions) { const taskDir = await getTaskDirectoryPath(globalStoragePath, id) @@ -112,6 +115,7 @@ export async function taskMetadata({ mode, ...(typeof apiConfigName === "string" && apiConfigName.length > 0 ? { apiConfigName } : {}), ...(initialStatus && { status: initialStatus }), + ...(taskPermissions && { taskPermissions }), } return { historyItem, tokenUsage } diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index df7acbdcb75..a4d836af8b8 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -41,6 +41,7 @@ import { getApiProtocol, type TaskPermissions, mergeTaskPermissions, + toTaskPermissions, getModelId, isRetiredProvider, isIdleAsk, @@ -480,8 +481,13 @@ export class Task extends EventEmitter implements TaskLike { this.parentTaskId = historyItem ? historyItem.parentTaskId : parentTask?.taskId this.childTaskId = undefined - // Merge task permissions with parent (most-restrictive-wins) - this.taskPermissions = mergeTaskPermissions(parentTask?.taskPermissions, taskPermissions) + // Merge task permissions with parent (most-restrictive-wins). + // When restoring from history, use the persisted permissions as the base; + // when creating fresh, use the permissions passed via new_task tool. + const effectivePermissions = historyItem?.taskPermissions + ? toTaskPermissions(historyItem.taskPermissions) + : taskPermissions + this.taskPermissions = mergeTaskPermissions(parentTask?.taskPermissions, effectivePermissions) this.metadata = { task: historyItem ? historyItem.task : task, @@ -1211,6 +1217,19 @@ export class Task extends EventEmitter implements TaskLike { await this.taskApiConfigReady } + // Serialize only the input-level permission fields for persistence + // (exclude internal _*PatternLayers fields which are runtime-only) + const persistablePermissions = this.taskPermissions + ? { + ...(this.taskPermissions.filePatterns && { filePatterns: this.taskPermissions.filePatterns }), + ...(this.taskPermissions.commandPatterns && { + commandPatterns: this.taskPermissions.commandPatterns, + }), + ...(this.taskPermissions.allowedTools && { allowedTools: this.taskPermissions.allowedTools }), + ...(this.taskPermissions.deniedTools && { deniedTools: this.taskPermissions.deniedTools }), + } + : undefined + const { historyItem, tokenUsage } = await taskMetadata({ taskId: this.taskId, rootTaskId: this.rootTaskId, @@ -1222,6 +1241,10 @@ export class Task extends EventEmitter implements TaskLike { mode: this._taskMode || defaultModeSlug, // Use the task's own mode, not the current provider mode. apiConfigName: this._taskApiConfigName, // Use the task's own provider profile, not the current provider profile. initialStatus: this.initialStatus, + taskPermissions: + persistablePermissions && Object.keys(persistablePermissions).length > 0 + ? persistablePermissions + : undefined, }) // Emit token/tool usage updates using debounced function From 2bc25eb4841d22584779295618997a36baf17b2f Mon Sep 17 00:00:00 2001 From: Roo Code Date: Tue, 12 May 2026 06:58:45 +0000 Subject: [PATCH 09/11] feat: add Orchestrator prompt guidance for permissions and UI visibility - Enhance Orchestrator customInstructions with guidance on using the permissions parameter (filePatterns, commandPatterns, allowedTools, deniedTools) including example use cases and most-restrictive-wins semantics explanation - Add permission boundaries display in the ChatRow newTask approval message so users can see what restrictions are being set before approving subtask creation - Add i18n translation keys for permission display - Add 8 new tests across packages/types and webview-ui --- .../orchestrator-permissions-prompt.spec.ts | 32 +++ packages/types/src/mode.ts | 2 +- packages/types/src/vscode-extension-host.ts | 7 + webview-ui/src/components/chat/ChatRow.tsx | 33 ++++ .../__tests__/ChatRow.permissions.spec.tsx | 182 ++++++++++++++++++ webview-ui/src/i18n/locales/en/chat.json | 7 +- 6 files changed, 261 insertions(+), 2 deletions(-) create mode 100644 packages/types/src/__tests__/orchestrator-permissions-prompt.spec.ts create mode 100644 webview-ui/src/components/chat/__tests__/ChatRow.permissions.spec.tsx diff --git a/packages/types/src/__tests__/orchestrator-permissions-prompt.spec.ts b/packages/types/src/__tests__/orchestrator-permissions-prompt.spec.ts new file mode 100644 index 00000000000..1a6a9bcc3ae --- /dev/null +++ b/packages/types/src/__tests__/orchestrator-permissions-prompt.spec.ts @@ -0,0 +1,32 @@ +import { describe, it, expect } from "vitest" +import { DEFAULT_MODES } from "../mode.js" +import type { ModeConfig } from "../mode.js" + +describe("Orchestrator mode - permissions prompt guidance", () => { + const orchestratorMode = DEFAULT_MODES.find((m: ModeConfig) => m.slug === "orchestrator") + + it("should have the orchestrator mode defined", () => { + expect(orchestratorMode).toBeDefined() + }) + + it("should include permissions guidance in customInstructions", () => { + expect(orchestratorMode!.customInstructions).toContain("permissions") + expect(orchestratorMode!.customInstructions).toContain("filePatterns") + expect(orchestratorMode!.customInstructions).toContain("commandPatterns") + expect(orchestratorMode!.customInstructions).toContain("allowedTools") + expect(orchestratorMode!.customInstructions).toContain("deniedTools") + }) + + it("should mention most-restrictive-wins semantics", () => { + expect(orchestratorMode!.customInstructions).toContain("most-restrictive-wins") + }) + + it("should provide example use cases for permissions", () => { + // Guidance about restricting file access + expect(orchestratorMode!.customInstructions).toContain("specific directory") + // Guidance about read-only research tasks + expect(orchestratorMode!.customInstructions).toContain("read-only research") + // Guidance about blocking shell access + expect(orchestratorMode!.customInstructions).toContain("shell access") + }) +}) diff --git a/packages/types/src/mode.ts b/packages/types/src/mode.ts index f981ba7bf9a..11f9a9c5da9 100644 --- a/packages/types/src/mode.ts +++ b/packages/types/src/mode.ts @@ -222,6 +222,6 @@ export const DEFAULT_MODES: readonly ModeConfig[] = [ description: "Coordinate tasks across multiple modes", groups: [], customInstructions: - "Your role is to coordinate complex workflows by delegating tasks to specialized modes. As an orchestrator, you should:\n\n1. When given a complex task, break it down into logical subtasks that can be delegated to appropriate specialized modes.\n\n2. For each subtask, use the `new_task` tool to delegate. Choose the most appropriate mode for the subtask's specific goal and provide comprehensive instructions in the `message` parameter. These instructions must include:\n * All necessary context from the parent task or previous subtasks required to complete the work.\n * A clearly defined scope, specifying exactly what the subtask should accomplish.\n * An explicit statement that the subtask should *only* perform the work outlined in these instructions and not deviate.\n * An instruction for the subtask to signal completion by using the `attempt_completion` tool, providing a concise yet thorough summary of the outcome in the `result` parameter, keeping in mind that this summary will be the source of truth used to keep track of what was completed on this project.\n * A statement that these specific instructions supersede any conflicting general instructions the subtask's mode might have.\n\n3. Track and manage the progress of all subtasks. When a subtask is completed, analyze its results and determine the next steps.\n\n4. Help the user understand how the different subtasks fit together in the overall workflow. Provide clear reasoning about why you're delegating specific tasks to specific modes.\n\n5. When all subtasks are completed, synthesize the results and provide a comprehensive overview of what was accomplished.\n\n6. Ask clarifying questions when necessary to better understand how to break down complex tasks effectively.\n\n7. Suggest improvements to the workflow based on the results of completed subtasks.\n\nUse subtasks to maintain clarity. If a request significantly shifts focus or requires a different expertise (mode), consider creating a subtask rather than overloading the current one.", + 'Your role is to coordinate complex workflows by delegating tasks to specialized modes. As an orchestrator, you should:\n\n1. When given a complex task, break it down into logical subtasks that can be delegated to appropriate specialized modes.\n\n2. For each subtask, use the `new_task` tool to delegate. Choose the most appropriate mode for the subtask\'s specific goal and provide comprehensive instructions in the `message` parameter. These instructions must include:\n * All necessary context from the parent task or previous subtasks required to complete the work.\n * A clearly defined scope, specifying exactly what the subtask should accomplish.\n * An explicit statement that the subtask should *only* perform the work outlined in these instructions and not deviate.\n * An instruction for the subtask to signal completion by using the `attempt_completion` tool, providing a concise yet thorough summary of the outcome in the `result` parameter, keeping in mind that this summary will be the source of truth used to keep track of what was completed on this project.\n * A statement that these specific instructions supersede any conflicting general instructions the subtask\'s mode might have.\n\n3. Track and manage the progress of all subtasks. When a subtask is completed, analyze its results and determine the next steps.\n\n4. Help the user understand how the different subtasks fit together in the overall workflow. Provide clear reasoning about why you\'re delegating specific tasks to specific modes.\n\n5. When all subtasks are completed, synthesize the results and provide a comprehensive overview of what was accomplished.\n\n6. Ask clarifying questions when necessary to better understand how to break down complex tasks effectively.\n\n7. Suggest improvements to the workflow based on the results of completed subtasks.\n\n8. When delegating subtasks, consider using the optional `permissions` parameter on `new_task` to restrict what the subtask can do. This is especially useful when:\n * The subtask should only modify files in a specific directory (use `filePatterns`, e.g. `["src/components/.*"]`).\n * The subtask should only run certain commands (use `commandPatterns`, e.g. `["npm test.*", "npm run lint"]`).\n * The subtask should be limited to specific tools (use `allowedTools`, e.g. `["read_file", "search_files"]` for read-only research tasks).\n * Certain tools should be explicitly blocked (use `deniedTools`, e.g. `["execute_command"]` to prevent shell access).\n Permissions are enforced at runtime and follow most-restrictive-wins semantics when subtasks are nested. Use them to keep subtasks focused and safe.\n\nUse subtasks to maintain clarity. If a request significantly shifts focus or requires a different expertise (mode), consider creating a subtask rather than overloading the current one.', }, ] as const diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index d8d2bab0f82..52f07493c1a 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -795,6 +795,13 @@ export interface ClineSayTool { description?: string // Properties for skill tool skill?: string + // Properties for newTask tool - permission boundaries set by parent + permissions?: { + filePatterns?: string[] + commandPatterns?: string[] + allowedTools?: string[] + deniedTools?: string[] + } } export interface ClineAskUseMcpServer { diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index ec02fda04cd..3c71d1e96ca 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -869,6 +869,39 @@ export const ChatRowContent = ({
+ {tool.permissions && ( +
+
{t("chat:subtasks.permissionBoundaries")}
+ {tool.permissions.filePatterns && ( +
+ {t("chat:subtasks.permissionFilePatterns", { + patterns: tool.permissions.filePatterns.join(", "), + })} +
+ )} + {tool.permissions.commandPatterns && ( +
+ {t("chat:subtasks.permissionCommandPatterns", { + patterns: tool.permissions.commandPatterns.join(", "), + })} +
+ )} + {tool.permissions.allowedTools && ( +
+ {t("chat:subtasks.permissionAllowedTools", { + tools: tool.permissions.allowedTools.join(", "), + })} +
+ )} + {tool.permissions.deniedTools && ( +
+ {t("chat:subtasks.permissionDeniedTools", { + tools: tool.permissions.deniedTools.join(", "), + })} +
+ )} +
+ )}
{childTaskId && !isFollowedBySubtaskResult && (