Skip to content

Commit 1a0f8fc

Browse files
PierrunoYTPierrunoYT
authored
fix(task): keep delegated child mode isolated (#1637)
* fix(task): keep delegated child mode isolated * test(task): complete task-mode doubles * test(task): prove task-local mode consumers * test(task): cover isolated same-mode rejection
1 parent f6af57a commit 1a0f8fc

16 files changed

Lines changed: 119 additions & 48 deletions

‎src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ describe("presentAssistantMessage - Custom Tool Recording", () => {
6767
didAlreadyUseTool: false,
6868
consecutiveMistakeCount: 0,
6969
clineMessages: [],
70+
getTaskMode: vi.fn().mockResolvedValue("code"),
7071
api: {
7172
getModel: () => ({ id: "test-model", info: {} }),
7273
},
@@ -86,7 +87,6 @@ describe("presentAssistantMessage - Custom Tool Recording", () => {
8687
}),
8788
}),
8889
},
89-
getTaskMode: vi.fn().mockResolvedValue("code"),
9090
say: vi.fn().mockResolvedValue(undefined),
9191
ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }),
9292
}
@@ -130,6 +130,37 @@ describe("presentAssistantMessage - Custom Tool Recording", () => {
130130
// Should record as "custom_tool", not "my_custom_tool"
131131
expect(mockTask.recordToolUsage).toHaveBeenCalledWith("custom_tool")
132132
})
133+
134+
it("passes the task-local mode to custom tool execution", async () => {
135+
mockTask.getTaskMode.mockResolvedValue("code")
136+
mockTask.providerRef.deref = () => ({
137+
getState: vi.fn().mockResolvedValue({
138+
mode: "orchestrator",
139+
customModes: [],
140+
experiments: { customTools: true },
141+
}),
142+
})
143+
mockTask.assistantMessageContent = [
144+
{
145+
type: "tool_use",
146+
id: "tool_call_task_mode",
147+
name: "my_custom_tool",
148+
params: {},
149+
partial: false,
150+
},
151+
]
152+
const execute = vi.fn().mockResolvedValue("Custom tool result")
153+
vi.mocked(customToolRegistry.has).mockReturnValue(true)
154+
vi.mocked(customToolRegistry.get).mockReturnValue({
155+
name: "my_custom_tool",
156+
description: "A custom tool",
157+
execute,
158+
})
159+
160+
await presentAssistantMessage(mockTask)
161+
162+
expect(execute).toHaveBeenCalledWith(undefined, { mode: "code", task: mockTask })
163+
})
133164
})
134165

135166
describe("Custom tool mode delegation regression", () => {

‎src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ describe("presentAssistantMessage - Image Handling in Native Tool Calling", () =
4242
didRejectTool: false,
4343
didAlreadyUseTool: false,
4444
consecutiveMistakeCount: 0,
45+
getTaskMode: vi.fn().mockResolvedValue("code"),
4546
api: {
4647
getModel: () => ({ id: "test-model", info: {} }),
4748
},
@@ -57,7 +58,6 @@ describe("presentAssistantMessage - Image Handling in Native Tool Calling", () =
5758
}),
5859
}),
5960
},
60-
getTaskMode: vi.fn().mockResolvedValue("code"),
6161
say: vi.fn().mockResolvedValue(undefined),
6262
ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }),
6363
}

‎src/core/assistant-message/__tests__/presentAssistantMessage-tool-usage-attribution.spec.ts‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// npx vitest src/core/assistant-message/__tests__/presentAssistantMessage-tool-usage-attribution.spec.ts
22

33
import type { Anthropic } from "@anthropic-ai/sdk"
4-
import { describe, it, expect, beforeEach, vi } from "vitest"
4+
import { describe, it, expect, beforeEach, vi, type Mock } from "vitest"
55
import { presentAssistantMessage } from "../presentAssistantMessage"
66
import { validateToolUse } from "../../tools/validateToolUse"
77
import { getModeBySlug } from "../../../shared/modes"
@@ -60,6 +60,7 @@ interface MockTask {
6060
didAlreadyUseTool: boolean
6161
consecutiveMistakeCount: number
6262
clineMessages: unknown[]
63+
getTaskMode: Mock<() => Promise<string>>
6364
api: { getModel: () => { id: string; info: Record<string, unknown> } }
6465
recordToolUsage: ReturnType<typeof vi.fn>
6566
recordToolError: ReturnType<typeof vi.fn>
@@ -75,7 +76,6 @@ interface MockTask {
7576
say: ReturnType<typeof vi.fn>
7677
ask: ReturnType<typeof vi.fn>
7778
pushToolResultToUserContent: ReturnType<typeof vi.fn>
78-
getTaskMode: ReturnType<typeof vi.fn>
7979
}
8080

8181
describe("presentAssistantMessage - tool usage attribution", () => {
@@ -99,6 +99,7 @@ describe("presentAssistantMessage - tool usage attribution", () => {
9999
didAlreadyUseTool: false,
100100
consecutiveMistakeCount: 0,
101101
clineMessages: [],
102+
getTaskMode: vi.fn().mockResolvedValue("code"),
102103
api: {
103104
getModel: () => ({ id: "test-model", info: {} }),
104105
},
@@ -118,7 +119,6 @@ describe("presentAssistantMessage - tool usage attribution", () => {
118119
say: vi.fn().mockResolvedValue(undefined),
119120
ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }),
120121
pushToolResultToUserContent: vi.fn(),
121-
getTaskMode: vi.fn().mockResolvedValue("code"),
122122
}
123123

124124
mockTask.pushToolResultToUserContent = vi
@@ -191,6 +191,35 @@ describe("presentAssistantMessage - tool usage attribution", () => {
191191
expect(mockTask.recordToolUsage).not.toHaveBeenCalledWith("mcp_")
192192
})
193193

194+
it("validates tools against the task-local mode when provider state differs", async () => {
195+
mockTask.getTaskMode.mockResolvedValue("code")
196+
mockTask.providerRef.deref = () => ({
197+
getState: vi.fn().mockResolvedValue({ mode: "orchestrator", customModes: [] }),
198+
})
199+
mockTask.assistantMessageContent = [
200+
{
201+
type: "tool_use",
202+
id: "call_task_mode",
203+
name: "read_file",
204+
params: { path: "test.txt" },
205+
nativeArgs: { path: "test.txt" },
206+
partial: false,
207+
},
208+
]
209+
210+
await presentAssistantMessage(mockTask as unknown as Task)
211+
212+
expect(validateToolUse).toHaveBeenCalledWith(
213+
"read_file",
214+
"code",
215+
[],
216+
{},
217+
{ path: "test.txt" },
218+
undefined,
219+
undefined,
220+
)
221+
})
222+
194223
it("records a safe failure key without leaking the raw tool name when validation fails", async () => {
195224
vi.mocked(validateToolUse).mockImplementation(() => {
196225
throw new Error('Tool "read_file" is not allowed in this mode.')

‎src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ describe("presentAssistantMessage - Unknown Tool Handling", () => {
4444
didAlreadyUseTool: false,
4545
consecutiveMistakeCount: 0,
4646
clineMessages: [],
47+
getTaskMode: vi.fn().mockResolvedValue("code"),
4748
api: {
4849
getModel: () => ({ id: "test-model", info: {} }),
4950
},
@@ -60,7 +61,6 @@ describe("presentAssistantMessage - Unknown Tool Handling", () => {
6061
}),
6162
}),
6263
},
63-
getTaskMode: vi.fn().mockResolvedValue("code"),
6464
say: vi.fn().mockResolvedValue(undefined),
6565
ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }),
6666
}

‎src/core/assistant-message/presentAssistantMessage.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ export async function presentAssistantMessage(cline: Task) {
343343
break
344344
}
345345

346-
// Fetch state early so it's available for toolDescription and validation
346+
// Shared provider state supplies global settings; mode is owned by the task.
347347
const state = await cline.providerRef.deref()?.getState()
348348
const { customModes, experiments: stateExperiments, disabledTools } = state ?? {}
349349
// Read the task-local mode, not the shared provider mode.

‎src/core/environment/__tests__/getEnvironmentDetails.spec.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ describe("getEnvironmentDetails", () => {
7575
terminalOutputLineLimit: 100,
7676
maxWorkspaceFiles: 50,
7777
maxOpenTabsContext: 10,
78-
mode: "code",
78+
mode: "orchestrator",
7979
customModes: [],
8080
experiments: {},
8181
customInstructions: "test instructions",
@@ -91,6 +91,7 @@ describe("getEnvironmentDetails", () => {
9191
cwd: mockCwd,
9292
taskId: mockTaskId,
9393
didEditFile: false,
94+
getTaskMode: vi.fn().mockResolvedValue("code"),
9495
fileContextTracker: {
9596
getAndClearRecentlyModifiedFiles: vi.fn().mockReturnValue([]),
9697
} as unknown as FileContextTracker,
@@ -117,7 +118,6 @@ describe("getEnvironmentDetails", () => {
117118
deref: vi.fn().mockReturnValue(mockProvider),
118119
[Symbol.toStringTag]: "WeakRef",
119120
} as unknown as WeakRef<ClineProvider>,
120-
getTaskMode: vi.fn().mockResolvedValue("code"),
121121
}
122122

123123
// Mock other dependencies.
@@ -157,6 +157,8 @@ describe("getEnvironmentDetails", () => {
157157

158158
expect(mockProvider.getState).toHaveBeenCalled()
159159

160+
expect(mockCline.getTaskMode).toHaveBeenCalled()
161+
expect(result).toContain("<slug>code</slug>")
160162
expect(getFullModeDetails).toHaveBeenCalledWith("code", [], undefined, {
161163
cwd: mockCwd,
162164
globalCustomInstructions: "test instructions",

‎src/core/environment/getEnvironmentDetails.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import delay from "delay"
88
import type { ExperimentId } from "@roo-code/types"
99

1010
import { formatLanguage } from "../../shared/language"
11-
import { defaultModeSlug, getFullModeDetails } from "../../shared/modes"
11+
import { getFullModeDetails } from "../../shared/modes"
1212
import { getApiMetrics } from "../../shared/getApiMetrics"
1313
import { listFiles } from "../../services/glob/list-files"
1414
import { TerminalRegistry } from "../../integrations/terminal/TerminalRegistry"

‎src/core/tools/RunSlashCommandTool.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export class RunSlashCommandTool extends BaseTool<"run_slash_command"> {
5555
const command = await getCommand(task.cwd, commandName)
5656

5757
if (!command) {
58-
const currentMode = state?.mode ?? "code"
58+
const currentMode = await task.getTaskMode()
5959
const skillsManager = provider?.getSkillsManager()
6060
const skillContent = await resolveSkillContentForMode(skillsManager, commandName, currentMode)
6161

‎src/core/tools/SkillTool.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,8 @@ export class SkillTool extends BaseTool<"skill"> {
4343
return
4444
}
4545

46-
// Get current mode for skill resolution
47-
const state = await provider?.getState()
48-
const currentMode = state?.mode ?? "code"
46+
// Resolve skills against the task's mode, not shared provider state.
47+
const currentMode = await task.getTaskMode()
4948

5049
// Fetch skill content
5150
const skillContent = await resolveSkillContentForMode(skillsManager, skillName, currentMode)

‎src/core/tools/SwitchModeTool.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import delay from "delay"
22

33
import { Task } from "../task/Task"
44
import { formatResponse } from "../prompts/responses"
5-
import { defaultModeSlug, getModeBySlug } from "../../shared/modes"
5+
import { getModeBySlug } from "../../shared/modes"
66
import { BaseTool, ToolCallbacks } from "./BaseTool"
77
import type { ToolUse } from "../../shared/tools"
88

@@ -38,8 +38,8 @@ export class SwitchModeTool extends BaseTool<"switch_mode"> {
3838
return
3939
}
4040

41-
// Check if already in requested mode
42-
const currentMode = (await task.providerRef.deref()?.getState())?.mode ?? defaultModeSlug
41+
// Mode belongs to the task; provider state may still reflect its parent.
42+
const currentMode = await task.getTaskMode()
4343

4444
if (currentMode === mode_slug) {
4545
task.recordToolError("switch_mode")

0 commit comments

Comments
 (0)