mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
feat: add optional todos parameter to new_task tool with experimental setting (#6329)
- Add optional todos parameter to new_task tool for hierarchical task planning - Implement experimental setting to optionally require todos parameter - Add clean state-based UI rendering to avoid spurious messages - Export and reuse parseMarkdownChecklist function - Add comprehensive test coverage for both optional and required modes - Maintain full backward compatibility (todos optional by default)
This commit is contained in:
parent
263e317ebd
commit
bb688dc8f8
15 changed files with 534 additions and 17 deletions
|
|
@ -6,7 +6,13 @@ import type { Keys, Equals, AssertEqual } from "./type-fu.js"
|
|||
* ExperimentId
|
||||
*/
|
||||
|
||||
export const experimentIds = ["powerSteering", "multiFileApplyDiff", "preventFocusDisruption", "assistantMessageParser"] as const
|
||||
export const experimentIds = [
|
||||
"powerSteering",
|
||||
"multiFileApplyDiff",
|
||||
"preventFocusDisruption",
|
||||
"assistantMessageParser",
|
||||
"newTaskRequireTodos",
|
||||
] as const
|
||||
|
||||
export const experimentIdsSchema = z.enum(experimentIds)
|
||||
|
||||
|
|
@ -21,6 +27,7 @@ export const experimentsSchema = z.object({
|
|||
multiFileApplyDiff: z.boolean().optional(),
|
||||
preventFocusDisruption: z.boolean().optional(),
|
||||
assistantMessageParser: z.boolean().optional(),
|
||||
newTaskRequireTodos: z.boolean().optional(),
|
||||
})
|
||||
|
||||
export type Experiments = z.infer<typeof experimentsSchema>
|
||||
|
|
|
|||
|
|
@ -2,22 +2,35 @@ import { ToolArgs } from "./types"
|
|||
|
||||
export function getNewTaskDescription(_args: ToolArgs): string {
|
||||
return `## new_task
|
||||
Description: This will let you create a new task instance in the chosen mode using your provided message.
|
||||
Description: This will let you create a new task instance in the chosen mode using your provided message and optional initial todo list.
|
||||
|
||||
Parameters:
|
||||
- mode: (required) The slug of the mode to start the new task in (e.g., "code", "debug", "architect").
|
||||
- message: (required) The initial user message or instructions for this new task.
|
||||
- todos: (optional by default, can be required via experimental setting) The initial todo list in markdown checklist format for the new task.
|
||||
Note: The 'todos' parameter can be configured to be required through the experimental setting 'newTaskRequireTodos'.
|
||||
|
||||
Usage:
|
||||
<new_task>
|
||||
<mode>your-mode-slug-here</mode>
|
||||
<message>Your initial instructions here</message>
|
||||
<todos>
|
||||
[ ] First task to complete
|
||||
[ ] Second task to complete
|
||||
[ ] Third task to complete
|
||||
</todos>
|
||||
</new_task>
|
||||
|
||||
Example:
|
||||
<new_task>
|
||||
<mode>code</mode>
|
||||
<message>Implement a new feature for the application.</message>
|
||||
<message>Implement user authentication</message>
|
||||
<todos>
|
||||
[ ] Set up auth middleware
|
||||
[ ] Create login endpoint
|
||||
[ ] Add session management
|
||||
[ ] Write tests
|
||||
</todos>
|
||||
</new_task>
|
||||
`
|
||||
}
|
||||
|
|
|
|||
|
|
@ -118,6 +118,7 @@ export type TaskOptions = {
|
|||
parentTask?: Task
|
||||
taskNumber?: number
|
||||
onCreated?: (task: Task) => void
|
||||
initialTodos?: TodoItem[]
|
||||
}
|
||||
|
||||
export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
||||
|
|
@ -268,6 +269,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
parentTask,
|
||||
taskNumber = -1,
|
||||
onCreated,
|
||||
initialTodos,
|
||||
}: TaskOptions) {
|
||||
super()
|
||||
|
||||
|
|
@ -345,11 +347,16 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
|
||||
this.toolRepetitionDetector = new ToolRepetitionDetector(this.consecutiveMistakeLimit)
|
||||
|
||||
// Initialize todo list if provided
|
||||
if (initialTodos && initialTodos.length > 0) {
|
||||
this.todoList = initialTodos
|
||||
}
|
||||
|
||||
onCreated?.(this)
|
||||
|
||||
if (startTask) {
|
||||
if (task || images) {
|
||||
this.startTask(task, images)
|
||||
this.startTask(task, images, initialTodos)
|
||||
} else if (historyItem) {
|
||||
this.resumeTaskFromHistory()
|
||||
} else {
|
||||
|
|
@ -930,7 +937,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
|
||||
// Start / Abort / Resume
|
||||
|
||||
private async startTask(task?: string, images?: string[]): Promise<void> {
|
||||
private async startTask(task?: string, images?: string[], initialTodos?: TodoItem[]): Promise<void> {
|
||||
// `conversationHistory` (for API) and `clineMessages` (for webview)
|
||||
// need to be in sync.
|
||||
// If the extension process were killed, then on restart the
|
||||
|
|
@ -939,9 +946,13 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
// messages from previous session).
|
||||
this.clineMessages = []
|
||||
this.apiConversationHistory = []
|
||||
await this.providerRef.deref()?.postStateToWebview()
|
||||
|
||||
// The todo list is already set in the constructor if initialTodos were provided
|
||||
// No need to add any messages - the todoList property is already set
|
||||
|
||||
await this.say("text", task, images)
|
||||
|
||||
await this.providerRef.deref()?.postStateToWebview()
|
||||
this.isInitialized = true
|
||||
|
||||
let imageBlocks: Anthropic.ImageBlockParam[] = formatResponse.imageBlocks(images)
|
||||
|
|
|
|||
|
|
@ -8,12 +8,48 @@ vi.mock("../../../shared/modes", () => ({
|
|||
defaultModeSlug: "ask",
|
||||
}))
|
||||
|
||||
vi.mock("../../../shared/experiments", () => ({
|
||||
experiments: {
|
||||
isEnabled: vi.fn(),
|
||||
},
|
||||
EXPERIMENT_IDS: {
|
||||
NEW_TASK_REQUIRE_TODOS: "newTaskRequireTodos",
|
||||
},
|
||||
}))
|
||||
|
||||
vi.mock("../../prompts/responses", () => ({
|
||||
formatResponse: {
|
||||
toolError: vi.fn((msg: string) => `Tool Error: ${msg}`),
|
||||
},
|
||||
}))
|
||||
|
||||
vi.mock("../updateTodoListTool", () => ({
|
||||
parseMarkdownChecklist: vi.fn((md: string) => {
|
||||
// Simple mock implementation
|
||||
const lines = md.split("\n").filter((line) => line.trim())
|
||||
return lines.map((line, index) => {
|
||||
let status = "pending"
|
||||
let content = line
|
||||
|
||||
if (line.includes("[x]") || line.includes("[X]")) {
|
||||
status = "completed"
|
||||
content = line.replace(/^\[x\]\s*/i, "")
|
||||
} else if (line.includes("[-]") || line.includes("[~]")) {
|
||||
status = "in_progress"
|
||||
content = line.replace(/^\[-\]\s*/, "").replace(/^\[~\]\s*/, "")
|
||||
} else {
|
||||
content = line.replace(/^\[\s*\]\s*/, "")
|
||||
}
|
||||
|
||||
return {
|
||||
id: `todo-${index}`,
|
||||
content,
|
||||
status,
|
||||
}
|
||||
})
|
||||
}),
|
||||
}))
|
||||
|
||||
// Define a minimal type for the resolved value
|
||||
type MockClineInstance = { taskId: string }
|
||||
|
||||
|
|
@ -22,7 +58,9 @@ const mockAskApproval = vi.fn<AskApproval>()
|
|||
const mockHandleError = vi.fn<HandleError>()
|
||||
const mockPushToolResult = vi.fn()
|
||||
const mockRemoveClosingTag = vi.fn((_name: string, value: string | undefined) => value ?? "")
|
||||
const mockInitClineWithTask = vi.fn<() => Promise<MockClineInstance>>().mockResolvedValue({ taskId: "mock-subtask-id" })
|
||||
const mockInitClineWithTask = vi
|
||||
.fn<(text?: string, images?: string[], parentTask?: any, options?: any) => Promise<MockClineInstance>>()
|
||||
.mockResolvedValue({ taskId: "mock-subtask-id" })
|
||||
const mockEmit = vi.fn()
|
||||
const mockRecordToolError = vi.fn()
|
||||
const mockSayAndCreateMissingParamError = vi.fn()
|
||||
|
|
@ -49,6 +87,7 @@ const mockCline = {
|
|||
import { newTaskTool } from "../newTaskTool"
|
||||
import type { ToolUse } from "../../../shared/tools"
|
||||
import { getModeBySlug } from "../../../shared/modes"
|
||||
import { experiments } from "../../../shared/experiments"
|
||||
|
||||
describe("newTaskTool", () => {
|
||||
beforeEach(() => {
|
||||
|
|
@ -63,6 +102,8 @@ describe("newTaskTool", () => {
|
|||
}) // Default valid mode
|
||||
mockCline.consecutiveMistakeCount = 0
|
||||
mockCline.isPaused = false
|
||||
// Default: experimental setting is disabled
|
||||
vi.mocked(experiments.isEnabled).mockReturnValue(false)
|
||||
})
|
||||
|
||||
it("should correctly un-escape \\\\@ to \\@ in the message passed to the new task", async () => {
|
||||
|
|
@ -72,6 +113,7 @@ describe("newTaskTool", () => {
|
|||
params: {
|
||||
mode: "code",
|
||||
message: "Review this: \\\\@file1.txt and also \\\\\\\\@file2.txt", // Input with \\@ and \\\\@
|
||||
todos: "[ ] First task\n[ ] Second task",
|
||||
},
|
||||
partial: false,
|
||||
}
|
||||
|
|
@ -93,6 +135,12 @@ describe("newTaskTool", () => {
|
|||
"Review this: \\@file1.txt and also \\\\\\@file2.txt", // Unit Test Expectation: \\@ -> \@, \\\\@ -> \\\\@
|
||||
undefined,
|
||||
mockCline,
|
||||
expect.objectContaining({
|
||||
initialTodos: expect.arrayContaining([
|
||||
expect.objectContaining({ content: "First task" }),
|
||||
expect.objectContaining({ content: "Second task" }),
|
||||
]),
|
||||
}),
|
||||
)
|
||||
|
||||
// Verify side effects
|
||||
|
|
@ -109,6 +157,7 @@ describe("newTaskTool", () => {
|
|||
params: {
|
||||
mode: "code",
|
||||
message: "This is already unescaped: \\@file1.txt",
|
||||
todos: "[ ] Test todo",
|
||||
},
|
||||
partial: false,
|
||||
}
|
||||
|
|
@ -126,6 +175,9 @@ describe("newTaskTool", () => {
|
|||
"This is already unescaped: \\@file1.txt", // Expected: \@ remains \@
|
||||
undefined,
|
||||
mockCline,
|
||||
expect.objectContaining({
|
||||
initialTodos: expect.any(Array),
|
||||
}),
|
||||
)
|
||||
})
|
||||
|
||||
|
|
@ -136,6 +188,7 @@ describe("newTaskTool", () => {
|
|||
params: {
|
||||
mode: "code",
|
||||
message: "A normal mention @file1.txt",
|
||||
todos: "[ ] Test todo",
|
||||
},
|
||||
partial: false,
|
||||
}
|
||||
|
|
@ -153,6 +206,9 @@ describe("newTaskTool", () => {
|
|||
"A normal mention @file1.txt", // Expected: @ remains @
|
||||
undefined,
|
||||
mockCline,
|
||||
expect.objectContaining({
|
||||
initialTodos: expect.any(Array),
|
||||
}),
|
||||
)
|
||||
})
|
||||
|
||||
|
|
@ -163,6 +219,7 @@ describe("newTaskTool", () => {
|
|||
params: {
|
||||
mode: "code",
|
||||
message: "Mix: @file0.txt, \\@file1.txt, \\\\@file2.txt, \\\\\\\\@file3.txt",
|
||||
todos: "[ ] Test todo",
|
||||
},
|
||||
partial: false,
|
||||
}
|
||||
|
|
@ -180,8 +237,367 @@ describe("newTaskTool", () => {
|
|||
"Mix: @file0.txt, \\@file1.txt, \\@file2.txt, \\\\\\@file3.txt", // Unit Test Expectation: @->@, \@->\@, \\@->\@, \\\\@->\\\\@
|
||||
undefined,
|
||||
mockCline,
|
||||
expect.objectContaining({
|
||||
initialTodos: expect.any(Array),
|
||||
}),
|
||||
)
|
||||
})
|
||||
|
||||
// Add more tests for error handling (missing params, invalid mode, approval denied) if needed
|
||||
it("should handle missing todos parameter gracefully (backward compatibility)", async () => {
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "new_task",
|
||||
params: {
|
||||
mode: "code",
|
||||
message: "Test message",
|
||||
// todos missing - should work for backward compatibility
|
||||
},
|
||||
partial: false,
|
||||
}
|
||||
|
||||
await newTaskTool(
|
||||
mockCline as any,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
mockRemoveClosingTag,
|
||||
)
|
||||
|
||||
// Should NOT error when todos is missing
|
||||
expect(mockSayAndCreateMissingParamError).not.toHaveBeenCalledWith("new_task", "todos")
|
||||
expect(mockCline.consecutiveMistakeCount).toBe(0)
|
||||
expect(mockCline.recordToolError).not.toHaveBeenCalledWith("new_task")
|
||||
|
||||
// Should create task with empty todos array
|
||||
expect(mockInitClineWithTask).toHaveBeenCalledWith(
|
||||
"Test message",
|
||||
undefined,
|
||||
mockCline,
|
||||
expect.objectContaining({
|
||||
initialTodos: [],
|
||||
}),
|
||||
)
|
||||
|
||||
// Should complete successfully
|
||||
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Successfully created new task"))
|
||||
})
|
||||
|
||||
it("should work with todos parameter when provided", async () => {
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "new_task",
|
||||
params: {
|
||||
mode: "code",
|
||||
message: "Test message with todos",
|
||||
todos: "[ ] First task\n[ ] Second task",
|
||||
},
|
||||
partial: false,
|
||||
}
|
||||
|
||||
await newTaskTool(
|
||||
mockCline as any,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
mockRemoveClosingTag,
|
||||
)
|
||||
|
||||
// Should parse and include todos when provided
|
||||
expect(mockInitClineWithTask).toHaveBeenCalledWith(
|
||||
"Test message with todos",
|
||||
undefined,
|
||||
mockCline,
|
||||
expect.objectContaining({
|
||||
initialTodos: expect.arrayContaining([
|
||||
expect.objectContaining({ content: "First task" }),
|
||||
expect.objectContaining({ content: "Second task" }),
|
||||
]),
|
||||
}),
|
||||
)
|
||||
|
||||
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Successfully created new task"))
|
||||
})
|
||||
|
||||
it("should error when mode parameter is missing", async () => {
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "new_task",
|
||||
params: {
|
||||
// mode missing
|
||||
message: "Test message",
|
||||
todos: "[ ] Test todo",
|
||||
},
|
||||
partial: false,
|
||||
}
|
||||
|
||||
await newTaskTool(
|
||||
mockCline as any,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
mockRemoveClosingTag,
|
||||
)
|
||||
|
||||
expect(mockSayAndCreateMissingParamError).toHaveBeenCalledWith("new_task", "mode")
|
||||
expect(mockCline.consecutiveMistakeCount).toBe(1)
|
||||
expect(mockCline.recordToolError).toHaveBeenCalledWith("new_task")
|
||||
})
|
||||
|
||||
it("should error when message parameter is missing", async () => {
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "new_task",
|
||||
params: {
|
||||
mode: "code",
|
||||
// message missing
|
||||
todos: "[ ] Test todo",
|
||||
},
|
||||
partial: false,
|
||||
}
|
||||
|
||||
await newTaskTool(
|
||||
mockCline as any,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
mockRemoveClosingTag,
|
||||
)
|
||||
|
||||
expect(mockSayAndCreateMissingParamError).toHaveBeenCalledWith("new_task", "message")
|
||||
expect(mockCline.consecutiveMistakeCount).toBe(1)
|
||||
expect(mockCline.recordToolError).toHaveBeenCalledWith("new_task")
|
||||
})
|
||||
|
||||
it("should parse todos with different statuses correctly", async () => {
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "new_task",
|
||||
params: {
|
||||
mode: "code",
|
||||
message: "Test message",
|
||||
todos: "[ ] Pending task\n[x] Completed task\n[-] In progress task",
|
||||
},
|
||||
partial: false,
|
||||
}
|
||||
|
||||
await newTaskTool(
|
||||
mockCline as any,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
mockRemoveClosingTag,
|
||||
)
|
||||
|
||||
expect(mockInitClineWithTask).toHaveBeenCalledWith(
|
||||
"Test message",
|
||||
undefined,
|
||||
mockCline,
|
||||
expect.objectContaining({
|
||||
initialTodos: expect.arrayContaining([
|
||||
expect.objectContaining({ content: "Pending task", status: "pending" }),
|
||||
expect.objectContaining({ content: "Completed task", status: "completed" }),
|
||||
expect.objectContaining({ content: "In progress task", status: "in_progress" }),
|
||||
]),
|
||||
}),
|
||||
)
|
||||
})
|
||||
|
||||
describe("experimental setting: newTaskRequireTodos", () => {
|
||||
it("should NOT require todos when experimental setting is disabled (default)", async () => {
|
||||
// Ensure experimental setting is disabled
|
||||
vi.mocked(experiments.isEnabled).mockReturnValue(false)
|
||||
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "new_task",
|
||||
params: {
|
||||
mode: "code",
|
||||
message: "Test message",
|
||||
// todos missing - should work when setting is disabled
|
||||
},
|
||||
partial: false,
|
||||
}
|
||||
|
||||
await newTaskTool(
|
||||
mockCline as any,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
mockRemoveClosingTag,
|
||||
)
|
||||
|
||||
// Should NOT error when todos is missing and setting is disabled
|
||||
expect(mockSayAndCreateMissingParamError).not.toHaveBeenCalledWith("new_task", "todos")
|
||||
expect(mockCline.consecutiveMistakeCount).toBe(0)
|
||||
expect(mockCline.recordToolError).not.toHaveBeenCalledWith("new_task")
|
||||
|
||||
// Should create task with empty todos array
|
||||
expect(mockInitClineWithTask).toHaveBeenCalledWith(
|
||||
"Test message",
|
||||
undefined,
|
||||
mockCline,
|
||||
expect.objectContaining({
|
||||
initialTodos: [],
|
||||
}),
|
||||
)
|
||||
|
||||
// Should complete successfully
|
||||
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Successfully created new task"))
|
||||
})
|
||||
|
||||
it("should REQUIRE todos when experimental setting is enabled", async () => {
|
||||
// Enable experimental setting
|
||||
vi.mocked(experiments.isEnabled).mockReturnValue(true)
|
||||
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "new_task",
|
||||
params: {
|
||||
mode: "code",
|
||||
message: "Test message",
|
||||
// todos missing - should error when setting is enabled
|
||||
},
|
||||
partial: false,
|
||||
}
|
||||
|
||||
await newTaskTool(
|
||||
mockCline as any,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
mockRemoveClosingTag,
|
||||
)
|
||||
|
||||
// Should error when todos is missing and setting is enabled
|
||||
expect(mockSayAndCreateMissingParamError).toHaveBeenCalledWith("new_task", "todos")
|
||||
expect(mockCline.consecutiveMistakeCount).toBe(1)
|
||||
expect(mockCline.recordToolError).toHaveBeenCalledWith("new_task")
|
||||
|
||||
// Should NOT create task
|
||||
expect(mockInitClineWithTask).not.toHaveBeenCalled()
|
||||
expect(mockPushToolResult).not.toHaveBeenCalledWith(
|
||||
expect.stringContaining("Successfully created new task"),
|
||||
)
|
||||
})
|
||||
|
||||
it("should work with todos when experimental setting is enabled", async () => {
|
||||
// Enable experimental setting
|
||||
vi.mocked(experiments.isEnabled).mockReturnValue(true)
|
||||
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "new_task",
|
||||
params: {
|
||||
mode: "code",
|
||||
message: "Test message",
|
||||
todos: "[ ] First task\n[ ] Second task",
|
||||
},
|
||||
partial: false,
|
||||
}
|
||||
|
||||
await newTaskTool(
|
||||
mockCline as any,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
mockRemoveClosingTag,
|
||||
)
|
||||
|
||||
// Should NOT error when todos is provided and setting is enabled
|
||||
expect(mockSayAndCreateMissingParamError).not.toHaveBeenCalledWith("new_task", "todos")
|
||||
expect(mockCline.consecutiveMistakeCount).toBe(0)
|
||||
|
||||
// Should create task with parsed todos
|
||||
expect(mockInitClineWithTask).toHaveBeenCalledWith(
|
||||
"Test message",
|
||||
undefined,
|
||||
mockCline,
|
||||
expect.objectContaining({
|
||||
initialTodos: expect.arrayContaining([
|
||||
expect.objectContaining({ content: "First task" }),
|
||||
expect.objectContaining({ content: "Second task" }),
|
||||
]),
|
||||
}),
|
||||
)
|
||||
|
||||
// Should complete successfully
|
||||
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Successfully created new task"))
|
||||
})
|
||||
|
||||
it("should work with empty todos string when experimental setting is enabled", async () => {
|
||||
// Enable experimental setting
|
||||
vi.mocked(experiments.isEnabled).mockReturnValue(true)
|
||||
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "new_task",
|
||||
params: {
|
||||
mode: "code",
|
||||
message: "Test message",
|
||||
todos: "", // Empty string should be accepted
|
||||
},
|
||||
partial: false,
|
||||
}
|
||||
|
||||
await newTaskTool(
|
||||
mockCline as any,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
mockRemoveClosingTag,
|
||||
)
|
||||
|
||||
// Should NOT error when todos is empty string and setting is enabled
|
||||
expect(mockSayAndCreateMissingParamError).not.toHaveBeenCalledWith("new_task", "todos")
|
||||
expect(mockCline.consecutiveMistakeCount).toBe(0)
|
||||
|
||||
// Should create task with empty todos array
|
||||
expect(mockInitClineWithTask).toHaveBeenCalledWith(
|
||||
"Test message",
|
||||
undefined,
|
||||
mockCline,
|
||||
expect.objectContaining({
|
||||
initialTodos: [],
|
||||
}),
|
||||
)
|
||||
|
||||
// Should complete successfully
|
||||
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Successfully created new task"))
|
||||
})
|
||||
|
||||
it("should check experimental setting with correct experiment ID", async () => {
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "new_task",
|
||||
params: {
|
||||
mode: "code",
|
||||
message: "Test message",
|
||||
},
|
||||
partial: false,
|
||||
}
|
||||
|
||||
await newTaskTool(
|
||||
mockCline as any,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
mockRemoveClosingTag,
|
||||
)
|
||||
|
||||
// Verify that experiments.isEnabled was called with correct experiment ID
|
||||
expect(experiments.isEnabled).toHaveBeenCalledWith(expect.any(Object), "newTaskRequireTodos")
|
||||
})
|
||||
})
|
||||
|
||||
// Add more tests for error handling (invalid mode, approval denied) if needed
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,12 +1,14 @@
|
|||
import delay from "delay"
|
||||
|
||||
import { RooCodeEventName } from "@roo-code/types"
|
||||
import { RooCodeEventName, TodoItem } from "@roo-code/types"
|
||||
|
||||
import { ToolUse, AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "../../shared/tools"
|
||||
import { Task } from "../task/Task"
|
||||
import { defaultModeSlug, getModeBySlug } from "../../shared/modes"
|
||||
import { formatResponse } from "../prompts/responses"
|
||||
import { t } from "../../i18n"
|
||||
import { parseMarkdownChecklist } from "./updateTodoListTool"
|
||||
import { experiments as Experiments, EXPERIMENT_IDS } from "../../shared/experiments"
|
||||
|
||||
export async function newTaskTool(
|
||||
cline: Task,
|
||||
|
|
@ -18,6 +20,7 @@ export async function newTaskTool(
|
|||
) {
|
||||
const mode: string | undefined = block.params.mode
|
||||
const message: string | undefined = block.params.message
|
||||
const todos: string | undefined = block.params.todos
|
||||
|
||||
try {
|
||||
if (block.partial) {
|
||||
|
|
@ -25,11 +28,13 @@ export async function newTaskTool(
|
|||
tool: "newTask",
|
||||
mode: removeClosingTag("mode", mode),
|
||||
content: removeClosingTag("message", message),
|
||||
todos: removeClosingTag("todos", todos),
|
||||
})
|
||||
|
||||
await cline.ask("tool", partialMessage, block.partial).catch(() => {})
|
||||
return
|
||||
} else {
|
||||
// Validate required parameters
|
||||
if (!mode) {
|
||||
cline.consecutiveMistakeCount++
|
||||
cline.recordToolError("new_task")
|
||||
|
|
@ -44,6 +49,33 @@ export async function newTaskTool(
|
|||
return
|
||||
}
|
||||
|
||||
// Get the experimental setting for requiring todos
|
||||
const provider = cline.providerRef.deref()
|
||||
const state = await provider?.getState()
|
||||
const requireTodos = Experiments.isEnabled(state?.experiments ?? {}, EXPERIMENT_IDS.NEW_TASK_REQUIRE_TODOS)
|
||||
|
||||
// Check if todos are required based on experimental setting
|
||||
// Note: undefined means not provided, empty string is valid
|
||||
if (requireTodos && todos === undefined) {
|
||||
cline.consecutiveMistakeCount++
|
||||
cline.recordToolError("new_task")
|
||||
pushToolResult(await cline.sayAndCreateMissingParamError("new_task", "todos"))
|
||||
return
|
||||
}
|
||||
|
||||
// Parse todos if provided, otherwise use empty array
|
||||
let todoItems: TodoItem[] = []
|
||||
if (todos) {
|
||||
try {
|
||||
todoItems = parseMarkdownChecklist(todos)
|
||||
} catch (error) {
|
||||
cline.consecutiveMistakeCount++
|
||||
cline.recordToolError("new_task")
|
||||
pushToolResult(formatResponse.toolError("Invalid todos format: must be a markdown checklist"))
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
cline.consecutiveMistakeCount = 0
|
||||
// Un-escape one level of backslashes before '@' for hierarchical subtasks
|
||||
// Un-escape one level: \\@ -> \@ (removes one backslash for hierarchical subtasks)
|
||||
|
|
@ -61,6 +93,7 @@ export async function newTaskTool(
|
|||
tool: "newTask",
|
||||
mode: targetMode.name,
|
||||
content: message,
|
||||
todos: todoItems,
|
||||
})
|
||||
|
||||
const didApprove = await askApproval("tool", toolMessage)
|
||||
|
|
@ -69,8 +102,7 @@ export async function newTaskTool(
|
|||
return
|
||||
}
|
||||
|
||||
const provider = cline.providerRef.deref()
|
||||
|
||||
// Re-get provider reference (we already have it from above)
|
||||
if (!provider) {
|
||||
return
|
||||
}
|
||||
|
|
@ -83,7 +115,9 @@ export async function newTaskTool(
|
|||
cline.pausedModeSlug = (await provider.getState()).mode ?? defaultModeSlug
|
||||
|
||||
// Create new task instance first (this preserves parent's current mode in its history)
|
||||
const newCline = await provider.initClineWithTask(unescapedMessage, undefined, cline)
|
||||
const newCline = await provider.initClineWithTask(unescapedMessage, undefined, cline, {
|
||||
initialTodos: todoItems,
|
||||
})
|
||||
if (!newCline) {
|
||||
pushToolResult(t("tools:newTask.errors.policy_restriction"))
|
||||
return
|
||||
|
|
@ -97,7 +131,9 @@ export async function newTaskTool(
|
|||
|
||||
cline.emit(RooCodeEventName.TaskSpawned, newCline.taskId)
|
||||
|
||||
pushToolResult(`Successfully created new task in ${targetMode.name} mode with message: ${unescapedMessage}`)
|
||||
pushToolResult(
|
||||
`Successfully created new task in ${targetMode.name} mode with message: ${unescapedMessage} and ${todoItems.length} todo items`,
|
||||
)
|
||||
|
||||
// Set the isPaused flag to true so the parent
|
||||
// task can wait for the sub-task to finish.
|
||||
|
|
|
|||
|
|
@ -100,7 +100,7 @@ function normalizeStatus(status: string | undefined): TodoStatus {
|
|||
return "pending"
|
||||
}
|
||||
|
||||
function parseMarkdownChecklist(md: string): TodoItem[] {
|
||||
export function parseMarkdownChecklist(md: string): TodoItem[] {
|
||||
if (typeof md !== "string") return []
|
||||
const lines = md
|
||||
.split(/\r?\n/)
|
||||
|
|
|
|||
|
|
@ -641,7 +641,12 @@ export class ClineProvider
|
|||
options: Partial<
|
||||
Pick<
|
||||
TaskOptions,
|
||||
"enableDiff" | "enableCheckpoints" | "fuzzyMatchThreshold" | "consecutiveMistakeLimit" | "experiments"
|
||||
| "enableDiff"
|
||||
| "enableCheckpoints"
|
||||
| "fuzzyMatchThreshold"
|
||||
| "consecutiveMistakeLimit"
|
||||
| "experiments"
|
||||
| "initialTodos"
|
||||
>
|
||||
> = {},
|
||||
) {
|
||||
|
|
@ -672,6 +677,7 @@ export class ClineProvider
|
|||
parentTask,
|
||||
taskNumber: this.clineStack.length + 1,
|
||||
onCreated: (instance) => this.emit(RooCodeEventName.TaskCreated, instance),
|
||||
initialTodos: options.initialTodos,
|
||||
...options,
|
||||
})
|
||||
|
||||
|
|
@ -1665,6 +1671,7 @@ export class ClineProvider
|
|||
? (taskHistory || []).find((item: HistoryItem) => item.id === this.getCurrentCline()?.taskId)
|
||||
: undefined,
|
||||
clineMessages: this.getCurrentCline()?.clineMessages || [],
|
||||
currentTaskTodos: this.getCurrentCline()?.todoList || [],
|
||||
taskHistory: (taskHistory || [])
|
||||
.filter((item: HistoryItem) => item.ts && item.task)
|
||||
.sort((a: HistoryItem, b: HistoryItem) => b.ts - a.ts),
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ import type {
|
|||
OrganizationAllowList,
|
||||
CloudUserInfo,
|
||||
ShareVisibility,
|
||||
TodoItem,
|
||||
} from "@roo-code/types"
|
||||
|
||||
import { GitCommit } from "../utils/git"
|
||||
|
|
@ -275,6 +276,7 @@ export type ExtensionState = Pick<
|
|||
version: string
|
||||
clineMessages: ClineMessage[]
|
||||
currentTaskItem?: HistoryItem
|
||||
currentTaskTodos?: TodoItem[] // Initial todos for the current task
|
||||
apiConfiguration?: ProviderSettings
|
||||
uriScheme?: string
|
||||
shouldShowAnnouncement: boolean
|
||||
|
|
|
|||
|
|
@ -30,6 +30,7 @@ describe("experiments", () => {
|
|||
multiFileApplyDiff: false,
|
||||
preventFocusDisruption: false,
|
||||
assistantMessageParser: false,
|
||||
newTaskRequireTodos: false,
|
||||
}
|
||||
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false)
|
||||
})
|
||||
|
|
@ -40,6 +41,7 @@ describe("experiments", () => {
|
|||
multiFileApplyDiff: false,
|
||||
preventFocusDisruption: false,
|
||||
assistantMessageParser: false,
|
||||
newTaskRequireTodos: false,
|
||||
}
|
||||
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(true)
|
||||
})
|
||||
|
|
@ -50,6 +52,7 @@ describe("experiments", () => {
|
|||
multiFileApplyDiff: false,
|
||||
preventFocusDisruption: false,
|
||||
assistantMessageParser: false,
|
||||
newTaskRequireTodos: false,
|
||||
}
|
||||
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ export const EXPERIMENT_IDS = {
|
|||
POWER_STEERING: "powerSteering",
|
||||
PREVENT_FOCUS_DISRUPTION: "preventFocusDisruption",
|
||||
ASSISTANT_MESSAGE_PARSER: "assistantMessageParser",
|
||||
NEW_TASK_REQUIRE_TODOS: "newTaskRequireTodos",
|
||||
} as const satisfies Record<string, ExperimentId>
|
||||
|
||||
type _AssertExperimentIds = AssertEqual<Equals<ExperimentId, Values<typeof EXPERIMENT_IDS>>>
|
||||
|
|
@ -20,6 +21,7 @@ export const experimentConfigsMap: Record<ExperimentKey, ExperimentConfig> = {
|
|||
POWER_STEERING: { enabled: false },
|
||||
PREVENT_FOCUS_DISRUPTION: { enabled: false },
|
||||
ASSISTANT_MESSAGE_PARSER: { enabled: false },
|
||||
NEW_TASK_REQUIRE_TODOS: { enabled: false },
|
||||
}
|
||||
|
||||
export const experimentDefault = Object.fromEntries(
|
||||
|
|
|
|||
|
|
@ -155,7 +155,7 @@ export interface SwitchModeToolUse extends ToolUse {
|
|||
|
||||
export interface NewTaskToolUse extends ToolUse {
|
||||
name: "new_task"
|
||||
params: Partial<Pick<Record<ToolParamName, string>, "mode" | "message">>
|
||||
params: Partial<Pick<Record<ToolParamName, string>, "mode" | "message" | "todos">>
|
||||
}
|
||||
|
||||
export interface SearchAndReplaceToolUse extends ToolUse {
|
||||
|
|
|
|||
|
|
@ -87,6 +87,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
|
|||
const {
|
||||
clineMessages: messages,
|
||||
currentTaskItem,
|
||||
currentTaskTodos,
|
||||
taskHistory,
|
||||
apiConfiguration,
|
||||
organizationAllowList,
|
||||
|
|
@ -144,8 +145,20 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
|
|||
const task = useMemo(() => messages.at(0), [messages])
|
||||
|
||||
const latestTodos = useMemo(() => {
|
||||
// First check if we have initial todos from the state (for new subtasks)
|
||||
if (currentTaskTodos && currentTaskTodos.length > 0) {
|
||||
// Check if there are any todo updates in messages
|
||||
const messageBasedTodos = getLatestTodo(messages)
|
||||
// If there are message-based todos, they take precedence (user has updated them)
|
||||
if (messageBasedTodos && messageBasedTodos.length > 0) {
|
||||
return messageBasedTodos
|
||||
}
|
||||
// Otherwise use the initial todos from state
|
||||
return currentTaskTodos
|
||||
}
|
||||
// Fall back to extracting from messages
|
||||
return getLatestTodo(messages)
|
||||
}, [messages])
|
||||
}, [messages, currentTaskTodos])
|
||||
|
||||
const modifiedMessages = useMemo(() => combineApiRequests(combineCommandSequences(messages.slice(1))), [messages])
|
||||
|
||||
|
|
|
|||
|
|
@ -31,6 +31,7 @@ export interface ExtensionStateContextType extends ExtensionState {
|
|||
mcpServers: McpServer[]
|
||||
hasSystemPromptOverride?: boolean
|
||||
currentCheckpoint?: string
|
||||
currentTaskTodos?: any[] // Initial todos for the current task
|
||||
filePaths: string[]
|
||||
openedTabs: Array<{ label: string; isActive: boolean; path?: string }>
|
||||
commands: Command[]
|
||||
|
|
|
|||
|
|
@ -230,6 +230,7 @@ describe("mergeExtensionState", () => {
|
|||
multiFileApplyDiff: true,
|
||||
preventFocusDisruption: false,
|
||||
assistantMessageParser: false,
|
||||
newTaskRequireTodos: false,
|
||||
} as Record<ExperimentId, boolean>,
|
||||
}
|
||||
|
||||
|
|
@ -248,6 +249,7 @@ describe("mergeExtensionState", () => {
|
|||
multiFileApplyDiff: true,
|
||||
preventFocusDisruption: false,
|
||||
assistantMessageParser: false,
|
||||
newTaskRequireTodos: false,
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -695,6 +695,10 @@
|
|||
"ASSISTANT_MESSAGE_PARSER": {
|
||||
"name": "Use new message parser",
|
||||
"description": "Enable the experimental streaming message parser that provides significant performance improvements for long assistant responses by processing messages more efficiently."
|
||||
},
|
||||
"NEW_TASK_REQUIRE_TODOS": {
|
||||
"name": "Require 'todos' list for new tasks",
|
||||
"description": "When enabled, the new_task tool will require a todos parameter to be provided. This ensures all new tasks start with a clear list of objectives. When disabled (default), the todos parameter remains optional for backward compatibility."
|
||||
}
|
||||
},
|
||||
"promptCaching": {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue