refactor: move newTaskRequireTodos from experimental to VSCode setting

- Added new VSCode configuration setting 'roo-cline.newTaskRequireTodos'
- Removed NEW_TASK_REQUIRE_TODOS from experimental settings
- Updated newTaskTool.ts to use VSCode configuration instead of experiments
- Updated prompt generation to use VSCode configuration
- Updated all related tests to use VSCode configuration mocks
- Removed experimental setting from all localization files
- Added new VSCode setting description to package.nls.json
This commit is contained in:
hannesrudolph 2025-08-06 17:29:37 -07:00
parent fbad1e9c11
commit 8a28e53520
14 changed files with 130 additions and 102 deletions

View file

@ -11,7 +11,6 @@ export const experimentIds = [
"multiFileApplyDiff",
"preventFocusDisruption",
"assistantMessageParser",
"newTaskRequireTodos",
] as const
export const experimentIdsSchema = z.enum(experimentIds)
@ -27,7 +26,6 @@ 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>

View file

@ -1,15 +1,33 @@
import { describe, it, expect } from "vitest"
import { describe, it, expect, vi, beforeEach } from "vitest"
import { getNewTaskDescription } from "../new-task"
import { ToolArgs } from "../types"
// Mock vscode module
vi.mock("vscode", () => ({
workspace: {
getConfiguration: vi.fn(() => ({
get: vi.fn(),
})),
},
}))
import * as vscode from "vscode"
describe("getNewTaskDescription", () => {
it("should show todos as optional when experiment is disabled", () => {
beforeEach(() => {
vi.clearAllMocks()
})
it("should show todos as optional when VSCode setting is disabled", () => {
const mockConfig = {
get: vi.fn().mockReturnValue(false),
}
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
const args: ToolArgs = {
cwd: "/test",
supportsComputerUse: false,
experiments: {
newTaskRequireTodos: false,
},
experiments: {},
}
const description = getNewTaskDescription(args)
@ -20,15 +38,22 @@ describe("getNewTaskDescription", () => {
// Should not contain any mention of required
expect(description).not.toContain("todos: (required)")
// Verify VSCode configuration was checked
expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith("roo-cline")
expect(mockConfig.get).toHaveBeenCalledWith("newTaskRequireTodos", false)
})
it("should show todos as required when experiment is enabled", () => {
it("should show todos as required when VSCode setting is enabled", () => {
const mockConfig = {
get: vi.fn().mockReturnValue(true),
}
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
const args: ToolArgs = {
cwd: "/test",
supportsComputerUse: false,
experiments: {
newTaskRequireTodos: true,
},
experiments: {},
}
const description = getNewTaskDescription(args)
@ -40,23 +65,18 @@ describe("getNewTaskDescription", () => {
// Should not contain any mention of optional for todos
expect(description).not.toContain("todos: (optional)")
expect(description).not.toContain("optional initial todo list")
// Verify VSCode configuration was checked
expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith("roo-cline")
expect(mockConfig.get).toHaveBeenCalledWith("newTaskRequireTodos", false)
})
it("should default to optional when experiments is undefined", () => {
const args: ToolArgs = {
cwd: "/test",
supportsComputerUse: false,
experiments: undefined,
it("should default to optional when VSCode setting returns undefined", () => {
const mockConfig = {
get: vi.fn().mockReturnValue(undefined),
}
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
const description = getNewTaskDescription(args)
// Check that todos is marked as optional by default
expect(description).toContain("todos: (optional)")
expect(description).toContain("optional initial todo list")
})
it("should default to optional when newTaskRequireTodos is undefined", () => {
const args: ToolArgs = {
cwd: "/test",
supportsComputerUse: false,
@ -71,24 +91,27 @@ describe("getNewTaskDescription", () => {
})
it("should always include the example with todos", () => {
const argsWithExperimentOff: ToolArgs = {
// Test with setting off
const mockConfigOff = {
get: vi.fn().mockReturnValue(false),
}
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfigOff as any)
const args: ToolArgs = {
cwd: "/test",
supportsComputerUse: false,
experiments: {
newTaskRequireTodos: false,
},
experiments: {},
}
const argsWithExperimentOn: ToolArgs = {
cwd: "/test",
supportsComputerUse: false,
experiments: {
newTaskRequireTodos: true,
},
}
const descriptionOff = getNewTaskDescription(args)
const descriptionOff = getNewTaskDescription(argsWithExperimentOff)
const descriptionOn = getNewTaskDescription(argsWithExperimentOn)
// Test with setting on
const mockConfigOn = {
get: vi.fn().mockReturnValue(true),
}
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfigOn as any)
const descriptionOn = getNewTaskDescription(args)
// Both should include the example with todos
const examplePattern = /<todos>\s*\[\s*\]\s*Set up auth middleware/s

View file

@ -1,7 +1,8 @@
import * as vscode from "vscode"
import { ToolArgs } from "./types"
export function getNewTaskDescription(args: ToolArgs): string {
const todosRequired = args.experiments?.newTaskRequireTodos === true
const todosRequired = vscode.workspace.getConfiguration("roo-cline").get<boolean>("newTaskRequireTodos", false)
const todosStatus = todosRequired ? "(required)" : "(optional)"
return `## new_task

View file

@ -8,12 +8,11 @@ vi.mock("../../../shared/modes", () => ({
defaultModeSlug: "ask",
}))
vi.mock("../../../shared/experiments", () => ({
experiments: {
isEnabled: vi.fn(),
},
EXPERIMENT_IDS: {
NEW_TASK_REQUIRE_TODOS: "newTaskRequireTodos",
vi.mock("vscode", () => ({
workspace: {
getConfiguration: vi.fn(() => ({
get: vi.fn(),
})),
},
}))
@ -87,7 +86,7 @@ const mockCline = {
import { newTaskTool } from "../newTaskTool"
import type { ToolUse } from "../../../shared/tools"
import { getModeBySlug } from "../../../shared/modes"
import { experiments } from "../../../shared/experiments"
import * as vscode from "vscode"
describe("newTaskTool", () => {
beforeEach(() => {
@ -102,8 +101,11 @@ describe("newTaskTool", () => {
}) // Default valid mode
mockCline.consecutiveMistakeCount = 0
mockCline.isPaused = false
// Default: experimental setting is disabled
vi.mocked(experiments.isEnabled).mockReturnValue(false)
// Default: VSCode setting is disabled
const mockConfig = {
get: vi.fn().mockReturnValue(false),
}
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
})
it("should correctly un-escape \\\\@ to \\@ in the message passed to the new task", async () => {
@ -407,10 +409,13 @@ describe("newTaskTool", () => {
)
})
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)
describe("VSCode setting: newTaskRequireTodos", () => {
it("should NOT require todos when VSCode setting is disabled (default)", async () => {
// Ensure VSCode setting is disabled
const mockConfig = {
get: vi.fn().mockReturnValue(false),
}
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
const block: ToolUse = {
type: "tool_use",
@ -432,7 +437,7 @@ describe("newTaskTool", () => {
mockRemoveClosingTag,
)
// Should NOT error when todos is missing and setting is disabled
// Should NOT error when todos is missing and VSCode setting is disabled
expect(mockSayAndCreateMissingParamError).not.toHaveBeenCalledWith("new_task", "todos")
expect(mockCline.consecutiveMistakeCount).toBe(0)
expect(mockCline.recordToolError).not.toHaveBeenCalledWith("new_task")
@ -451,9 +456,12 @@ describe("newTaskTool", () => {
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)
it("should REQUIRE todos when VSCode setting is enabled", async () => {
// Enable VSCode setting
const mockConfig = {
get: vi.fn().mockReturnValue(true),
}
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
const block: ToolUse = {
type: "tool_use",
@ -475,7 +483,7 @@ describe("newTaskTool", () => {
mockRemoveClosingTag,
)
// Should error when todos is missing and setting is enabled
// Should error when todos is missing and VSCode setting is enabled
expect(mockSayAndCreateMissingParamError).toHaveBeenCalledWith("new_task", "todos")
expect(mockCline.consecutiveMistakeCount).toBe(1)
expect(mockCline.recordToolError).toHaveBeenCalledWith("new_task")
@ -487,9 +495,12 @@ describe("newTaskTool", () => {
)
})
it("should work with todos when experimental setting is enabled", async () => {
// Enable experimental setting
vi.mocked(experiments.isEnabled).mockReturnValue(true)
it("should work with todos when VSCode setting is enabled", async () => {
// Enable VSCode setting
const mockConfig = {
get: vi.fn().mockReturnValue(true),
}
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
const block: ToolUse = {
type: "tool_use",
@ -511,7 +522,7 @@ describe("newTaskTool", () => {
mockRemoveClosingTag,
)
// Should NOT error when todos is provided and setting is enabled
// Should NOT error when todos is provided and VSCode setting is enabled
expect(mockSayAndCreateMissingParamError).not.toHaveBeenCalledWith("new_task", "todos")
expect(mockCline.consecutiveMistakeCount).toBe(0)
@ -532,9 +543,12 @@ describe("newTaskTool", () => {
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)
it("should work with empty todos string when VSCode setting is enabled", async () => {
// Enable VSCode setting
const mockConfig = {
get: vi.fn().mockReturnValue(true),
}
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
const block: ToolUse = {
type: "tool_use",
@ -556,7 +570,7 @@ describe("newTaskTool", () => {
mockRemoveClosingTag,
)
// Should NOT error when todos is empty string and setting is enabled
// Should NOT error when todos is empty string and VSCode setting is enabled
expect(mockSayAndCreateMissingParamError).not.toHaveBeenCalledWith("new_task", "todos")
expect(mockCline.consecutiveMistakeCount).toBe(0)
@ -574,7 +588,13 @@ describe("newTaskTool", () => {
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Successfully created new task"))
})
it("should check experimental setting with correct experiment ID", async () => {
it("should check VSCode configuration with correct setting name", async () => {
const mockGet = vi.fn().mockReturnValue(false)
const mockConfig = {
get: mockGet,
}
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
const block: ToolUse = {
type: "tool_use",
name: "new_task",
@ -594,8 +614,9 @@ describe("newTaskTool", () => {
mockRemoveClosingTag,
)
// Verify that experiments.isEnabled was called with correct experiment ID
expect(experiments.isEnabled).toHaveBeenCalledWith(expect.any(Object), "newTaskRequireTodos")
// Verify that VSCode configuration was checked with correct setting name
expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith("roo-cline")
expect(mockGet).toHaveBeenCalledWith("newTaskRequireTodos", false)
})
})

View file

@ -1,4 +1,5 @@
import delay from "delay"
import * as vscode from "vscode"
import { RooCodeEventName, TodoItem } from "@roo-code/types"
@ -8,7 +9,6 @@ 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,
@ -49,14 +49,10 @@ export async function newTaskTool(
return
}
// Get the experimental setting for requiring todos
const provider = cline.providerRef.deref()
if (!provider) {
pushToolResult(formatResponse.toolError("Provider reference lost"))
return
}
const state = await provider.getState()
const requireTodos = Experiments.isEnabled(state?.experiments ?? {}, EXPERIMENT_IDS.NEW_TASK_REQUIRE_TODOS)
// Get the VSCode configuration setting for requiring todos
const requireTodos = vscode.workspace
.getConfiguration("roo-cline")
.get<boolean>("newTaskRequireTodos", false)
// Check if todos are required based on experimental setting
// Note: undefined means not provided, empty string is valid
@ -85,6 +81,14 @@ export async function newTaskTool(
// Un-escape one level: \\@ -> \@ (removes one backslash for hierarchical subtasks)
const unescapedMessage = message.replace(/\\\\@/g, "\\@")
// Get provider and state for mode verification
const provider = cline.providerRef.deref()
if (!provider) {
pushToolResult(formatResponse.toolError("Provider reference lost"))
return
}
const state = await provider.getState()
// Verify the mode exists
const targetMode = getModeBySlug(mode, state?.customModes)

View file

@ -391,6 +391,11 @@
"type": "boolean",
"default": true,
"description": "%settings.useAgentRules.description%"
},
"roo-cline.newTaskRequireTodos": {
"type": "boolean",
"default": false,
"description": "%settings.newTaskRequireTodos.description%"
}
}
}

View file

@ -37,5 +37,6 @@
"settings.customStoragePath.description": "Custom storage path. Leave empty to use the default location. Supports absolute paths (e.g. 'D:\\RooCodeStorage')",
"settings.enableCodeActions.description": "Enable Roo Code quick fixes",
"settings.autoImportSettingsPath.description": "Path to a RooCode configuration file to automatically import on extension startup. Supports absolute paths and paths relative to the home directory (e.g. '~/Documents/roo-code-settings.json'). Leave empty to disable auto-import.",
"settings.useAgentRules.description": "Enable loading of AGENTS.md files for agent-specific rules (see https://agent-rules.org/)"
"settings.useAgentRules.description": "Enable loading of AGENTS.md files for agent-specific rules (see https://agent-rules.org/)",
"settings.newTaskRequireTodos.description": "Require todos parameter when creating new tasks with the new_task tool"
}

View file

@ -30,7 +30,6 @@ describe("experiments", () => {
multiFileApplyDiff: false,
preventFocusDisruption: false,
assistantMessageParser: false,
newTaskRequireTodos: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false)
})
@ -41,7 +40,6 @@ describe("experiments", () => {
multiFileApplyDiff: false,
preventFocusDisruption: false,
assistantMessageParser: false,
newTaskRequireTodos: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(true)
})
@ -52,7 +50,6 @@ describe("experiments", () => {
multiFileApplyDiff: false,
preventFocusDisruption: false,
assistantMessageParser: false,
newTaskRequireTodos: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false)
})

View file

@ -5,7 +5,6 @@ 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>>>
@ -21,7 +20,6 @@ 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(

View file

@ -696,10 +696,6 @@
"ASSISTANT_MESSAGE_PARSER": {
"name": "Utilitza el nou analitzador de missatges",
"description": "Activa l'analitzador de missatges en streaming experimental que millora el rendiment en respostes llargues processant els missatges de manera més eficient."
},
"NEW_TASK_REQUIRE_TODOS": {
"name": "Requerir la llista 'todos' per a noves tasques",
"description": "Quan estigui activat, l'eina new_task requerirà que es proporcioni un paràmetre 'todos'. Això garanteix que totes les noves tasques comencin amb una llista clara d'objectius. Quan estigui desactivat (per defecte), el paràmetre 'todos' continua sent opcional per a la compatibilitat amb versions anteriors."
}
},
"promptCaching": {

View file

@ -696,10 +696,6 @@
"ASSISTANT_MESSAGE_PARSER": {
"name": "Neuen Nachrichtenparser verwenden",
"description": "Aktiviere den experimentellen Streaming-Nachrichtenparser, der lange Antworten durch effizientere Verarbeitung spürbar schneller macht."
},
"NEW_TASK_REQUIRE_TODOS": {
"name": "'todos'-Liste für neue Aufgaben anfordern",
"description": "Wenn aktiviert, erfordert das new_task-Tool die Angabe eines todos-Parameters. Dies stellt sicher, dass alle neuen Aufgaben mit einer klaren Zielliste beginnen. Wenn deaktiviert (Standard), bleibt der todos-Parameter aus Gründen der Abwärtskompatibilität optional."
}
},
"promptCaching": {

View file

@ -695,10 +695,6 @@
"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": {

View file

@ -696,10 +696,6 @@
"ASSISTANT_MESSAGE_PARSER": {
"name": "Usar el nuevo analizador de mensajes",
"description": "Activa el analizador de mensajes en streaming experimental que mejora el rendimiento en respuestas largas procesando los mensajes de forma más eficiente."
},
"NEW_TASK_REQUIRE_TODOS": {
"name": "Requerir lista de 'todos' para nuevas tareas",
"description": "Cuando está habilitado, la herramienta new_task requerirá que se proporcione un parámetro todos. Esto asegura que todas las nuevas tareas comiencen con una lista clara de objetivos. Cuando está deshabilitado (predeterminado), el parámetro todos permanece opcional por compatibilidad con versiones anteriores."
}
},
"promptCaching": {

View file

@ -696,10 +696,6 @@
"ASSISTANT_MESSAGE_PARSER": {
"name": "Utiliser le nouveau parseur de messages",
"description": "Active le parseur de messages en streaming expérimental qui accélère nettement les longues réponses en traitant les messages plus efficacement."
},
"NEW_TASK_REQUIRE_TODOS": {
"name": "Exiger la liste 'todos' pour les nouvelles tâches",
"description": "Lorsqu'il est activé, l'outil new_task exigera qu'un paramètre todos soit fourni. Cela garantit que toutes les nouvelles tâches commencent avec une liste claire d'objectifs. Lorsqu'il est désactivé (par défaut), le paramètre todos reste facultatif pour la compatibilité descendante."
}
},
"promptCaching": {