From f406cf709ec95e3475702abe93649d132a180f6b Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Sun, 16 Aug 2026 17:05:09 +0800 Subject: [PATCH] feat(task): auto-flatten subtasks inline when the nesting limit is reached --- src/core/task/Task.ts | 27 +++ src/core/tools/AttemptCompletionTool.ts | 10 + src/core/tools/NewTaskTool.ts | 35 ++++ .../tools/__tests__/inlineSubtask.spec.ts | 146 +++++++++++++ .../__tests__/newTaskInlineFlatten.spec.ts | 191 ++++++++++++++++++ src/core/tools/inlineSubtask.ts | 82 ++++++++ 6 files changed, 491 insertions(+) create mode 100644 src/core/tools/__tests__/inlineSubtask.spec.ts create mode 100644 src/core/tools/__tests__/newTaskInlineFlatten.spec.ts create mode 100644 src/core/tools/inlineSubtask.ts diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index cc5f316253..8dfad1d4e0 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -142,6 +142,24 @@ const DEFAULT_USAGE_COLLECTION_TIMEOUT_MS = 5000 // 5 seconds const FORCED_CONTEXT_REDUCTION_PERCENT = 75 // Keep 75% of context (remove 25%) on context window errors const MAX_CONTEXT_WINDOW_RETRIES = 3 // Maximum retries for context window errors +/** + * In-memory phase marker for an auto-flattened inline subtask. + * + * When `new_task` would exceed `maxNestingDepth` and `autoFlattenOnLimit` is set, + * the subtask is NOT opened as a child Task. Instead the parent Task records this + * marker and executes the instruction inline in its own conversation (the tool_result + * doubles as the inline prompt). The marker is cleared when the inline phase completes + * (`attempt_completion`) or the task is aborted/cancelled. + * + * Deliberately NOT persisted: inline state is a transient execution phase, not lineage. + */ +export interface InlineSubtask { + /** The subtask instruction to execute inline. */ + message: string + /** Parsed todos for the subtask (empty when none were provided). */ + todos: TodoItem[] +} + export interface TaskOptions extends CreateTaskOptions { provider: ClineProvider apiConfiguration: ProviderSettings @@ -178,6 +196,11 @@ export class Task extends EventEmitter implements TaskLike { * resumed without their live parent — the provider backfills those before first save. */ readonly depthAuthoritative: boolean + /** + * Set while an auto-flattened subtask is executing inline in this task's own + * conversation. Cleared on completion or abort. Never persisted. + */ + inlineSubtask?: InlineSubtask pendingNewTaskToolCallId?: string readonly instanceId: string @@ -2276,6 +2299,10 @@ export class Task extends EventEmitter implements TaskLike { this.abort = true + // Clear any in-flight inline subtask phase so a cancelled task resumes as an + // ordinary parent conversation with no orphaned marker. + this.inlineSubtask = undefined + // Reset consecutive error counters on abort (manual intervention) this.consecutiveNoToolUseCount = 0 this.consecutiveNoAssistantMessagesCount = 0 diff --git a/src/core/tools/AttemptCompletionTool.ts b/src/core/tools/AttemptCompletionTool.ts index b5f19decb0..1750fdf1b5 100644 --- a/src/core/tools/AttemptCompletionTool.ts +++ b/src/core/tools/AttemptCompletionTool.ts @@ -78,6 +78,16 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> { task.consecutiveMistakeCount = 0 + // Inline subtask phase completion (auto-flattened): clear the marker and let the + // loop continue with a tool_result. No askFinishSubTaskApproval — the user is + // already watching this same conversation, so an approval popup would be double + // friction. This task itself continues as before; it has NOT completed. + if (task.inlineSubtask) { + task.inlineSubtask = undefined + pushToolResult(`[inline subtask completed]\n${result}\nThe parent conversation continues.`) + return + } + await task.say("completion_result", result, undefined, false) // Whether this attempt_completion call is a stale replay of an already-completed diff --git a/src/core/tools/NewTaskTool.ts b/src/core/tools/NewTaskTool.ts index f36d8e1e37..b5716f70d8 100644 --- a/src/core/tools/NewTaskTool.ts +++ b/src/core/tools/NewTaskTool.ts @@ -2,7 +2,10 @@ import * as vscode from "vscode" import { TodoItem } from "@roo-code/types" +import { DEFAULT_AUTO_FLATTEN_ON_LIMIT, DEFAULT_MAX_NESTING_DEPTH } from "@roo-code/types" + import { Task } from "../task/Task" +import { decideInlineFlatten } from "./inlineSubtask" import { getModeBySlug } from "../../shared/modes" import { formatResponse } from "../prompts/responses" import { t } from "../../i18n" @@ -96,6 +99,38 @@ export class NewTaskTool extends BaseTool<"new_task"> { return } + // Auto-flatten inline decision (depth check BEFORE any approval prompt). + // When the subtask would exceed maxNestingDepth and autoFlattenOnLimit is set, + // it runs inline in this task's own conversation instead of opening a child tab. + const maxNestingDepth = state?.maxNestingDepth ?? DEFAULT_MAX_NESTING_DEPTH + const autoFlattenOnLimit = state?.autoFlattenOnLimit ?? DEFAULT_AUTO_FLATTEN_ON_LIMIT + const decision = decideInlineFlatten({ + childDepth: task.depth + 1, + maxNestingDepth, + autoFlattenOnLimit, + inlineActive: task.inlineSubtask !== undefined, + message: unescapedMessage, + todos: todoItems, + }) + + if (decision.action === "reject-nested") { + pushToolResult(formatResponse.toolError(decision.message)) + return + } + + if (decision.action === "flatten") { + // Set the phase marker and let the tool_result double as the inline prompt. + task.inlineSubtask = { message: unescapedMessage, todos: todoItems } + pushToolResult(decision.directive) + return + } + + if (decision.action === "reject-limit") { + pushToolResult(formatResponse.toolError(decision.message)) + return + } + + // decision.action === "delegate" — normal flow unchanged. const toolMessage = JSON.stringify({ tool: "newTask", mode: targetMode.name, diff --git a/src/core/tools/__tests__/inlineSubtask.spec.ts b/src/core/tools/__tests__/inlineSubtask.spec.ts new file mode 100644 index 0000000000..bbc9b16077 --- /dev/null +++ b/src/core/tools/__tests__/inlineSubtask.spec.ts @@ -0,0 +1,146 @@ +// npx vitest run core/tools/__tests__/inlineSubtask.spec.ts + +import { describe, it, expect } from "vitest" +import type { TodoItem } from "@roo-code/types" +import { decideInlineFlatten, buildInlineDirective } from "../inlineSubtask" + +const todos: TodoItem[] = [ + { content: "step one", status: "pending" }, + { content: "step two", status: "completed" }, +] as unknown as TodoItem[] + +describe("decideInlineFlatten (pure decision)", () => { + it("delegates normally when within the limit", () => { + const d = decideInlineFlatten({ + childDepth: 2, + maxNestingDepth: 2, + autoFlattenOnLimit: true, + inlineActive: false, + message: "m", + todos, + }) + expect(d).toEqual({ action: "delegate" }) + }) + + it("delegates when exactly at the limit (childDepth === max)", () => { + const d = decideInlineFlatten({ + childDepth: 2, + maxNestingDepth: 2, + autoFlattenOnLimit: true, + inlineActive: false, + message: "m", + todos, + }) + expect(d.action).toBe("delegate") + }) + + it("flattens when over the limit and autoFlattenOnLimit is true", () => { + const d = decideInlineFlatten({ + childDepth: 3, + maxNestingDepth: 2, + autoFlattenOnLimit: true, + inlineActive: false, + message: "do X", + todos, + }) + expect(d.action).toBe("flatten") + if (d.action === "flatten") { + expect(d.directive).toContain("auto-flattened") + expect(d.directive).toContain("nesting limit 2 reached") + expect(d.directive).toContain("do X") + } + }) + + it("rejects when over the limit and autoFlattenOnLimit is false", () => { + const d = decideInlineFlatten({ + childDepth: 3, + maxNestingDepth: 2, + autoFlattenOnLimit: false, + inlineActive: false, + message: "m", + todos, + }) + expect(d.action).toBe("reject-limit") + if (d.action === "reject-limit") { + expect(d.message).toContain("auto-flatten is disabled") + } + }) + + it("treats maxNestingDepth 0 as delegation-disabled → always flatten when over", () => { + const d = decideInlineFlatten({ + childDepth: 1, + maxNestingDepth: 0, + autoFlattenOnLimit: true, + inlineActive: false, + message: "m", + todos, + }) + expect(d.action).toBe("flatten") + }) + + it("rejects a nested new_task while an inline phase is active (precedence over delegate)", () => { + const d = decideInlineFlatten({ + childDepth: 1, + maxNestingDepth: 5, + autoFlattenOnLimit: true, + inlineActive: true, + message: "m", + todos, + }) + expect(d.action).toBe("reject-nested") + }) + + it("rejects a nested new_task while active even when over the limit (nested wins)", () => { + const d = decideInlineFlatten({ + childDepth: 9, + maxNestingDepth: 2, + autoFlattenOnLimit: true, + inlineActive: true, + message: "m", + todos, + }) + expect(d.action).toBe("reject-nested") + }) + + it("includes todos in the flatten directive when present", () => { + const d = decideInlineFlatten({ + childDepth: 3, + maxNestingDepth: 2, + autoFlattenOnLimit: true, + inlineActive: false, + message: "m", + todos, + }) + if (d.action === "flatten") { + expect(d.directive).toContain("step one") + expect(d.directive).toContain("step two") + } + }) + + it("omits the Todos section when no todos are provided", () => { + const d = decideInlineFlatten({ + childDepth: 3, + maxNestingDepth: 2, + autoFlattenOnLimit: true, + inlineActive: false, + message: "m", + todos: [], + }) + if (d.action === "flatten") { + expect(d.directive).not.toContain("Todos:") + } + }) +}) + +describe("buildInlineDirective", () => { + it("embeds the instruction and todos", () => { + const dir = buildInlineDirective("fix the bug", todos, 2) + expect(dir).toContain("fix the bug") + expect(dir).toContain("step one") + }) + + it("instructs to call attempt_completion when done", () => { + const dir = buildInlineDirective("m", [], 3) + expect(dir).toContain("attempt_completion") + }) +}) diff --git a/src/core/tools/__tests__/newTaskInlineFlatten.spec.ts b/src/core/tools/__tests__/newTaskInlineFlatten.spec.ts new file mode 100644 index 0000000000..3c960966a9 --- /dev/null +++ b/src/core/tools/__tests__/newTaskInlineFlatten.spec.ts @@ -0,0 +1,191 @@ +// npx vitest run core/tools/__tests__/newTaskInlineFlatten.spec.ts + +import { describe, it, expect, vi } from "vitest" +import type { TodoItem } from "@roo-code/types" +import { Task } from "../../task/Task" +import type { InlineSubtask } from "../../task/Task" +import { newTaskTool } from "../NewTaskTool" +import { attemptCompletionTool } from "../AttemptCompletionTool" +import type { ToolCallbacks } from "../BaseTool" + +/** + * Minimal provider double. `getState` returns the taskTree settings; the delegate case + * additionally needs `delegateParentAndOpenChild`. Cast once to ClineProvider so the + * tool's `(provider as any).delegateParentAndOpenChild` call resolves. + */ +function makeProvider(overrides: { maxNestingDepth?: number; autoFlattenOnLimit?: boolean } = {}) { + const delegateParentAndOpenChild = vi.fn().mockResolvedValue({ taskId: "child-1" }) + return { + getState: vi.fn().mockResolvedValue({ + maxNestingDepth: overrides.maxNestingDepth ?? 2, + autoFlattenOnLimit: overrides.autoFlattenOnLimit ?? true, + }), + delegateParentAndOpenChild, + } +} + +/** Precise Task double carrying only the fields NewTaskTool/AttemptCompletionTool touch. */ +function makeTask(opts: { depth?: number; inlineSubtask?: InlineSubtask; provider: unknown }) { + // Build a plain double carrying only the fields NewTaskTool/AttemptCompletionTool touch, + // then cast once to Task (same pattern as new-task-delegation.spec.ts). A single + // `as unknown as Task` avoids per-field intersection-type conflicts with Task's real members. + const task = { + taskId: "parent-1", + depth: opts.depth ?? 0, + inlineSubtask: opts.inlineSubtask, + consecutiveMistakeCount: 0, + didToolFailInCurrentTurn: false, + recordToolError: vi.fn(), + sayAndCreateMissingParamError: vi.fn().mockResolvedValue("missing param"), + providerRef: { deref: () => opts.provider }, + } + return task as unknown as Task +} + +function makeCallbacks() { + const askApproval = vi.fn().mockResolvedValue(true) + const pushToolResult = vi.fn() + const handleError = vi.fn() + const callbacks: ToolCallbacks = { askApproval, handleError, pushToolResult } + return { askApproval, pushToolResult, handleError, callbacks } +} + +describe("NewTaskTool auto-flatten inline", () => { + it("delegates normally when within the limit (approval + child opened)", async () => { + const provider = makeProvider({ maxNestingDepth: 2 }) + const task = makeTask({ depth: 0, provider }) // child would be depth 1 <= 2 + const { askApproval, pushToolResult } = makeCallbacks() + + await newTaskTool.execute({ mode: "code", message: "do X" }, task, { + askApproval, + handleError: vi.fn(), + pushToolResult, + }) + + expect(askApproval).toHaveBeenCalledTimes(1) + expect(provider.delegateParentAndOpenChild).toHaveBeenCalledWith( + expect.objectContaining({ parentTaskId: expect.anything() }), + ) + expect(task.inlineSubtask).toBeUndefined() + // Delegation reflected in the tool result, not an inline directive. + expect(pushToolResult).toHaveBeenCalledWith("Delegated to child task child-1") + }) + + it("flattens inline when over the limit (no approval, no child, marker set)", async () => { + const provider = makeProvider({ maxNestingDepth: 2 }) + const task = makeTask({ depth: 2, provider }) // child would be depth 3 > 2 + const { askApproval, pushToolResult } = makeCallbacks() + + await newTaskTool.execute({ mode: "code", message: "do X" }, task, { + askApproval, + handleError: vi.fn(), + pushToolResult, + }) + + // No approval prompt and no child opened. + expect(askApproval).not.toHaveBeenCalled() + expect(provider.delegateParentAndOpenChild).not.toHaveBeenCalled() + // Phase marker set with the instruction. + expect(task.inlineSubtask).toEqual({ message: "do X", todos: [] }) + // tool_result doubles as the inline directive. + const pushed = pushToolResult.mock.calls[0][0] as string + expect(pushed).toContain("auto-flattened") + expect(pushed).toContain("do X") + }) + + it("rejects when over the limit and autoFlattenOnLimit is false (error result, no marker)", async () => { + const provider = makeProvider({ maxNestingDepth: 2, autoFlattenOnLimit: false }) + const task = makeTask({ depth: 2, provider }) // child would be depth 3 > 2 + const { askApproval, pushToolResult } = makeCallbacks() + + await newTaskTool.execute({ mode: "code", message: "do X" }, task, { + askApproval, + handleError: vi.fn(), + pushToolResult, + }) + + expect(askApproval).not.toHaveBeenCalled() + expect(provider.delegateParentAndOpenChild).not.toHaveBeenCalled() + expect(task.inlineSubtask).toBeUndefined() + const pushed = pushToolResult.mock.calls[0][0] as string + expect(pushed.toLowerCase()).toContain("error") + }) + + it("rejects a nested new_task while an inline phase is already active", async () => { + const provider = makeProvider({ maxNestingDepth: 5 }) + const task = makeTask({ depth: 1, provider, inlineSubtask: { message: "outer", todos: [] } }) + const { askApproval, pushToolResult } = makeCallbacks() + + await newTaskTool.execute({ mode: "code", message: "inner" }, task, { + askApproval, + handleError: vi.fn(), + pushToolResult, + }) + + expect(askApproval).not.toHaveBeenCalled() + expect(provider.delegateParentAndOpenChild).not.toHaveBeenCalled() + // Existing marker preserved (not overwritten by the rejected nested call). + expect(task.inlineSubtask?.message).toBe("outer") + const pushed = pushToolResult.mock.calls[0][0] as string + expect(pushed.toLowerCase()).toContain("error") + }) +}) + +describe("AttemptCompletionTool inline-phase completion", () => { + it("clears the marker, pushes a continue result, and skips askFinishSubTaskApproval", async () => { + const provider = makeProvider() + const task = makeTask({ depth: 2, provider, inlineSubtask: { message: "do X", todos: [] } }) + // AttemptCompletionTool reads todoList; leave it undefined so the open-todos guard is skipped. + ;(task as unknown as { todoList?: TodoItem[] }).todoList = undefined + + const askFinishSubTaskApproval = vi.fn().mockResolvedValue(true) + const pushToolResult = vi.fn() + const say = vi.fn().mockResolvedValue({ response: "yesButtonClicked" }) + ;(task as unknown as { say: typeof say }).say = say + + await attemptCompletionTool.execute({ result: "done with the subtask" }, task, { + askApproval: vi.fn().mockResolvedValue(true), + handleError: vi.fn(), + pushToolResult, + askFinishSubTaskApproval, + toolDescription: () => "attempt_completion", + }) + + // Marker cleared. + expect(task.inlineSubtask).toBeUndefined() + // No subtask-finish approval popup (user is already in this conversation). + expect(askFinishSubTaskApproval).not.toHaveBeenCalled() + // The loop continues with a tool_result summarizing the inline completion. + const pushed = pushToolResult.mock.calls[0][0] as string + expect(pushed).toContain("[inline subtask completed]") + expect(pushed).toContain("done with the subtask") + }) + + it("does NOT take the inline branch when no marker is set (falls through to normal flow)", async () => { + const provider = makeProvider() + const task = makeTask({ depth: 2, provider }) // no inlineSubtask + ;(task as unknown as { todoList?: TodoItem[] }).todoList = undefined + + const askFinishSubTaskApproval = vi.fn().mockResolvedValue(true) + const pushToolResult = vi.fn() + // Normal flow reaches task.ask("completion_result", ...); stub it to return a decline. + ;(task as unknown as { ask: ReturnType }).ask = vi + .fn() + .mockResolvedValue({ response: "noButtonClicked" }) + const say = vi.fn().mockResolvedValue(undefined) + ;(task as unknown as { say: typeof say }).say = say + + await attemptCompletionTool.execute({ result: "normal completion" }, task, { + askApproval: vi.fn().mockResolvedValue(true), + handleError: vi.fn(), + pushToolResult, + askFinishSubTaskApproval, + toolDescription: () => "attempt_completion", + }) + + // No inline marker was set, so the inline branch must not have fired. + expect(task.inlineSubtask).toBeUndefined() + const pushed = pushToolResult.mock.calls.map((c) => c[0]).join("\n") as string + expect(pushed).not.toContain("[inline subtask completed]") + }) +}) diff --git a/src/core/tools/inlineSubtask.ts b/src/core/tools/inlineSubtask.ts new file mode 100644 index 0000000000..ac7835eb8f --- /dev/null +++ b/src/core/tools/inlineSubtask.ts @@ -0,0 +1,82 @@ +import type { TodoItem } from "@roo-code/types" + +/** + * Inputs for the auto-flatten inline decision. + * + * The decision is a pure function so it can be unit-tested without a live Task or + * vscode host. `childDepth` is the depth a child Task would have if opened as a real + * task (`parent.depth + 1`). + */ +export interface InlineFlattenInput { + /** Depth the subtask would occupy if opened as a real child Task. */ + childDepth: number + /** Configured maximum nesting depth (root = 0). `0` disables delegation entirely. */ + maxNestingDepth: number + /** When true, an over-limit subtask runs inline instead of being rejected. */ + autoFlattenOnLimit: boolean + /** True when this task is already executing an inline subtask phase. */ + inlineActive: boolean + /** The subtask instruction (used to build the flatten directive). */ + message: string + /** Parsed todos for the subtask (empty when none were provided). */ + todos: TodoItem[] +} + +export type InlineFlattenDecision = + | { action: "reject-nested"; message: string } + | { action: "flatten"; directive: string } + | { action: "reject-limit"; message: string } + | { action: "delegate" } + +/** + * Decide how a `new_task` call should be handled given the current nesting depth and + * settings. Pure — no side effects, no Task/vscode access. + * + * Precedence: + * 1. A nested `new_task` while an inline phase is already active is rejected (P1 forbids + * recursion into a second inline subtask). + * 2. Within the limit → normal delegation flow (`delegate`). + * 3. Over the limit + `autoFlattenOnLimit` → flatten inline (`flatten`). + * 4. Over the limit + `!autoFlattenOnLimit` → reject so work continues directly. + */ +export function decideInlineFlatten(input: InlineFlattenInput): InlineFlattenDecision { + const { childDepth, maxNestingDepth, autoFlattenOnLimit, inlineActive, message, todos } = input + + if (inlineActive) { + return { + action: "reject-nested", + message: + "Cannot start a nested subtask while an inline subtask is already in progress. " + + "Complete the current inline subtask with attempt_completion first.", + } + } + + const overLimit = childDepth > maxNestingDepth + if (!overLimit) { + return { action: "delegate" } + } + + if (autoFlattenOnLimit) { + return { action: "flatten", directive: buildInlineDirective(message, todos, maxNestingDepth) } + } + + return { + action: "reject-limit", + message: + `Nesting limit ${maxNestingDepth} reached and auto-flatten is disabled. ` + + "Continue working directly in the current conversation instead of delegating.", + } +} + +/** Build the inline directive that doubles as the subtask prompt (zero synthetic messages). */ +export function buildInlineDirective(message: string, todos: TodoItem[], maxNestingDepth: number): string { + const todoText = todos.length > 0 ? `\nTodos:\n${todos.map((t) => `- [ ] ${t.content}`).join("\n")}` : "" + return ( + `[auto-flattened: nesting limit ${maxNestingDepth} reached — executing inline]\n` + + "You are now executing this subtask INLINE in the current conversation.\n" + + `Subtask instruction: ${message}` + + todoText + + "\nExecute it with your available tools. When done, call attempt_completion " + + "with a summary of what you did." + ) +}