Skip to content

Commit bbf71de

Browse files
author
Merge Resolver
committed
feat: move newTaskRequireTodos from experimental to VSCode settings
- Added newTaskRequireTodos as a VSCode configuration property in src/package.json - Added description in src/package.nls.json - Updated newTaskTool.ts to read from VSCode configuration instead of experiments - Removed NEW_TASK_REQUIRE_TODOS from experimental settings in src/shared/experiments.ts - Removed newTaskRequireTodos from packages/types/src/experiment.ts - Updated tests to use VSCode configuration mocking instead of experiments - Removed references from experiments test file - Maintains backward compatibility (defaults to false)
1 parent e9da4b9 commit bbf71de

File tree

7 files changed

+63
-40
lines changed

7 files changed

+63
-40
lines changed

packages/types/src/experiment.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ export const experimentIds = [
1111
"multiFileApplyDiff",
1212
"preventFocusDisruption",
1313
"assistantMessageParser",
14-
"newTaskRequireTodos",
1514
] as const
1615

1716
export const experimentIdsSchema = z.enum(experimentIds)
@@ -27,7 +26,6 @@ export const experimentsSchema = z.object({
2726
multiFileApplyDiff: z.boolean().optional(),
2827
preventFocusDisruption: z.boolean().optional(),
2928
assistantMessageParser: z.boolean().optional(),
30-
newTaskRequireTodos: z.boolean().optional(),
3129
})
3230

3331
export type Experiments = z.infer<typeof experimentsSchema>

src/core/tools/__tests__/newTaskTool.spec.ts

Lines changed: 50 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,21 @@
22

33
import type { AskApproval, HandleError } from "../../../shared/tools"
44

5+
// Mock vscode module
6+
vi.mock("vscode", () => ({
7+
workspace: {
8+
getConfiguration: vi.fn(() => ({
9+
get: vi.fn(),
10+
})),
11+
},
12+
}))
13+
514
// Mock other modules first - these are hoisted to the top
615
vi.mock("../../../shared/modes", () => ({
716
getModeBySlug: vi.fn(),
817
defaultModeSlug: "ask",
918
}))
1019

11-
vi.mock("../../../shared/experiments", () => ({
12-
experiments: {
13-
isEnabled: vi.fn(),
14-
},
15-
EXPERIMENT_IDS: {
16-
NEW_TASK_REQUIRE_TODOS: "newTaskRequireTodos",
17-
},
18-
}))
19-
2020
vi.mock("../../prompts/responses", () => ({
2121
formatResponse: {
2222
toolError: vi.fn((msg: string) => `Tool Error: ${msg}`),
@@ -87,7 +87,7 @@ const mockCline = {
8787
import { newTaskTool } from "../newTaskTool"
8888
import type { ToolUse } from "../../../shared/tools"
8989
import { getModeBySlug } from "../../../shared/modes"
90-
import { experiments } from "../../../shared/experiments"
90+
import * as vscode from "vscode"
9191

9292
describe("newTaskTool", () => {
9393
beforeEach(() => {
@@ -102,8 +102,11 @@ describe("newTaskTool", () => {
102102
}) // Default valid mode
103103
mockCline.consecutiveMistakeCount = 0
104104
mockCline.isPaused = false
105-
// Default: experimental setting is disabled
106-
vi.mocked(experiments.isEnabled).mockReturnValue(false)
105+
// Default: VSCode setting is disabled
106+
const mockGet = vi.fn().mockReturnValue(false)
107+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({
108+
get: mockGet,
109+
} as any)
107110
})
108111

109112
it("should correctly un-escape \\\\@ to \\@ in the message passed to the new task", async () => {
@@ -407,10 +410,13 @@ describe("newTaskTool", () => {
407410
)
408411
})
409412

410-
describe("experimental setting: newTaskRequireTodos", () => {
411-
it("should NOT require todos when experimental setting is disabled (default)", async () => {
412-
// Ensure experimental setting is disabled
413-
vi.mocked(experiments.isEnabled).mockReturnValue(false)
413+
describe("VSCode setting: newTaskRequireTodos", () => {
414+
it("should NOT require todos when VSCode setting is disabled (default)", async () => {
415+
// Ensure VSCode setting is disabled
416+
const mockGet = vi.fn().mockReturnValue(false)
417+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({
418+
get: mockGet,
419+
} as any)
414420

415421
const block: ToolUse = {
416422
type: "tool_use",
@@ -451,9 +457,12 @@ describe("newTaskTool", () => {
451457
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Successfully created new task"))
452458
})
453459

454-
it("should REQUIRE todos when experimental setting is enabled", async () => {
455-
// Enable experimental setting
456-
vi.mocked(experiments.isEnabled).mockReturnValue(true)
460+
it("should REQUIRE todos when VSCode setting is enabled", async () => {
461+
// Enable VSCode setting
462+
const mockGet = vi.fn().mockReturnValue(true)
463+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({
464+
get: mockGet,
465+
} as any)
457466

458467
const block: ToolUse = {
459468
type: "tool_use",
@@ -487,9 +496,12 @@ describe("newTaskTool", () => {
487496
)
488497
})
489498

490-
it("should work with todos when experimental setting is enabled", async () => {
491-
// Enable experimental setting
492-
vi.mocked(experiments.isEnabled).mockReturnValue(true)
499+
it("should work with todos when VSCode setting is enabled", async () => {
500+
// Enable VSCode setting
501+
const mockGet = vi.fn().mockReturnValue(true)
502+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({
503+
get: mockGet,
504+
} as any)
493505

494506
const block: ToolUse = {
495507
type: "tool_use",
@@ -532,9 +544,12 @@ describe("newTaskTool", () => {
532544
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Successfully created new task"))
533545
})
534546

535-
it("should work with empty todos string when experimental setting is enabled", async () => {
536-
// Enable experimental setting
537-
vi.mocked(experiments.isEnabled).mockReturnValue(true)
547+
it("should work with empty todos string when VSCode setting is enabled", async () => {
548+
// Enable VSCode setting
549+
const mockGet = vi.fn().mockReturnValue(true)
550+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({
551+
get: mockGet,
552+
} as any)
538553

539554
const block: ToolUse = {
540555
type: "tool_use",
@@ -574,7 +589,13 @@ describe("newTaskTool", () => {
574589
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Successfully created new task"))
575590
})
576591

577-
it("should check experimental setting with correct experiment ID", async () => {
592+
it("should check VSCode setting with correct configuration key", async () => {
593+
const mockGet = vi.fn().mockReturnValue(false)
594+
const mockGetConfiguration = vi.fn().mockReturnValue({
595+
get: mockGet,
596+
} as any)
597+
vi.mocked(vscode.workspace.getConfiguration).mockImplementation(mockGetConfiguration)
598+
578599
const block: ToolUse = {
579600
type: "tool_use",
580601
name: "new_task",
@@ -594,8 +615,9 @@ describe("newTaskTool", () => {
594615
mockRemoveClosingTag,
595616
)
596617

597-
// Verify that experiments.isEnabled was called with correct experiment ID
598-
expect(experiments.isEnabled).toHaveBeenCalledWith(expect.any(Object), "newTaskRequireTodos")
618+
// Verify that VSCode configuration was accessed correctly
619+
expect(mockGetConfiguration).toHaveBeenCalledWith("roo-cline")
620+
expect(mockGet).toHaveBeenCalledWith("newTaskRequireTodos", false)
599621
})
600622
})
601623

src/core/tools/newTaskTool.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import delay from "delay"
2+
import * as vscode from "vscode"
23

34
import { RooCodeEventName, TodoItem } from "@roo-code/types"
45

@@ -8,7 +9,6 @@ import { defaultModeSlug, getModeBySlug } from "../../shared/modes"
89
import { formatResponse } from "../prompts/responses"
910
import { t } from "../../i18n"
1011
import { parseMarkdownChecklist } from "./updateTodoListTool"
11-
import { experiments as Experiments, EXPERIMENT_IDS } from "../../shared/experiments"
1212

1313
export async function newTaskTool(
1414
cline: Task,
@@ -49,16 +49,18 @@ export async function newTaskTool(
4949
return
5050
}
5151

52-
// Get the experimental setting for requiring todos
52+
// Get the VSCode setting for requiring todos
5353
const provider = cline.providerRef.deref()
5454
if (!provider) {
5555
pushToolResult(formatResponse.toolError("Provider reference lost"))
5656
return
5757
}
5858
const state = await provider.getState()
59-
const requireTodos = Experiments.isEnabled(state?.experiments ?? {}, EXPERIMENT_IDS.NEW_TASK_REQUIRE_TODOS)
59+
const requireTodos = vscode.workspace
60+
.getConfiguration("roo-cline")
61+
.get<boolean>("newTaskRequireTodos", false)
6062

61-
// Check if todos are required based on experimental setting
63+
// Check if todos are required based on VSCode setting
6264
// Note: undefined means not provided, empty string is valid
6365
if (requireTodos && todos === undefined) {
6466
cline.consecutiveMistakeCount++

src/package.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,11 @@
398398
"minimum": 0,
399399
"maximum": 3600,
400400
"description": "%settings.apiRequestTimeout.description%"
401+
},
402+
"roo-cline.newTaskRequireTodos": {
403+
"type": "boolean",
404+
"default": false,
405+
"description": "%settings.newTaskRequireTodos.description%"
401406
}
402407
}
403408
}

src/package.nls.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,5 +39,6 @@
3939
"settings.enableCodeActions.description": "Enable Roo Code quick fixes",
4040
"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.",
4141
"settings.useAgentRules.description": "Enable loading of AGENTS.md files for agent-specific rules (see https://agent-rules.org/)",
42-
"settings.apiRequestTimeout.description": "Maximum time in seconds to wait for API responses (0 = no timeout, 1-3600s, default: 600s). Higher values are recommended for local providers like LM Studio and Ollama that may need more processing time."
42+
"settings.apiRequestTimeout.description": "Maximum time in seconds to wait for API responses (0 = no timeout, 1-3600s, default: 600s). Higher values are recommended for local providers like LM Studio and Ollama that may need more processing time.",
43+
"settings.newTaskRequireTodos.description": "Require todos parameter when creating new tasks with the new_task tool"
4344
}

src/shared/__tests__/experiments.spec.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ describe("experiments", () => {
3030
multiFileApplyDiff: false,
3131
preventFocusDisruption: false,
3232
assistantMessageParser: false,
33-
newTaskRequireTodos: false,
3433
}
3534
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false)
3635
})
@@ -41,7 +40,6 @@ describe("experiments", () => {
4140
multiFileApplyDiff: false,
4241
preventFocusDisruption: false,
4342
assistantMessageParser: false,
44-
newTaskRequireTodos: false,
4543
}
4644
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(true)
4745
})
@@ -52,7 +50,6 @@ describe("experiments", () => {
5250
multiFileApplyDiff: false,
5351
preventFocusDisruption: false,
5452
assistantMessageParser: false,
55-
newTaskRequireTodos: false,
5653
}
5754
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false)
5855
})

src/shared/experiments.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ export const EXPERIMENT_IDS = {
55
POWER_STEERING: "powerSteering",
66
PREVENT_FOCUS_DISRUPTION: "preventFocusDisruption",
77
ASSISTANT_MESSAGE_PARSER: "assistantMessageParser",
8-
NEW_TASK_REQUIRE_TODOS: "newTaskRequireTodos",
98
} as const satisfies Record<string, ExperimentId>
109

1110
type _AssertExperimentIds = AssertEqual<Equals<ExperimentId, Values<typeof EXPERIMENT_IDS>>>
@@ -21,7 +20,6 @@ export const experimentConfigsMap: Record<ExperimentKey, ExperimentConfig> = {
2120
POWER_STEERING: { enabled: false },
2221
PREVENT_FOCUS_DISRUPTION: { enabled: false },
2322
ASSISTANT_MESSAGE_PARSER: { enabled: false },
24-
NEW_TASK_REQUIRE_TODOS: { enabled: false },
2523
}
2624

2725
export const experimentDefault = Object.fromEntries(

0 commit comments

Comments
 (0)