Skip to content

Commit fa561b1

Browse files
committed
fix(delegation): isolate parent/child provider state during fan-out
1 parent 0cd7dea commit fa561b1

10 files changed

Lines changed: 909 additions & 75 deletions

File tree

apps/vscode-e2e/src/fixtures/subtasks.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ const SUBTASK_XPROFILE_SAME_CHILD_MARKER = "SUBTASK_CHILD_SAME_PROFILE"
1616
const SUBTASK_XPROFILE_DIFFERENT_CHILD_MARKER = "SUBTASK_CHILD_DIFFERENT_PROFILE"
1717
const SUBTASK_FANOUT_PARENT_MARKER = "SUBTASK_PARENT_FANOUT_CONCURRENT"
1818
const SUBTASK_FANOUT_CHILD_MARKER = "SUBTASK_CHILD_FANOUT_CONCURRENT"
19+
const SUBTASK_FANOUT_XPROFILE_PARENT_MARKER = "SUBTASK_PARENT_FANOUT_CROSS_PROFILE"
20+
const SUBTASK_FANOUT_XPROFILE_CHILD_MARKER = "SUBTASK_CHILD_FANOUT_CROSS_PROFILE"
1921

2022
const SUBTASK_CHILD_PROMPT = `${SUBTASK_CHILD_MARKER}: Ask the user exactly this follow-up question: What is the square root of 81? After the user answers, complete with only the answer.`
2123
export const SUBTASK_PARENT_PROMPT = `${SUBTASK_PARENT_MARKER}: Use the new_task tool exactly once. Create an ask-mode subtask with this exact message: "${SUBTASK_CHILD_PROMPT}" Do not answer directly.`
@@ -27,6 +29,12 @@ export const SUBTASK_FANOUT_PARENT_FOLLOWUP = "Parent fan-out is still active?"
2729
export const SUBTASK_FANOUT_CHILD_RESULT = "Fan-out child completed"
2830
const SUBTASK_FANOUT_CHILD_PROMPT = `${SUBTASK_FANOUT_CHILD_MARKER}: Complete with the exact result "${SUBTASK_FANOUT_CHILD_RESULT}".`
2931
export const SUBTASK_FANOUT_PARENT_PROMPT = `${SUBTASK_FANOUT_PARENT_MARKER}: Use the new_task tool exactly once. Create an ask-mode subtask with this exact message: "${SUBTASK_FANOUT_CHILD_PROMPT}" After delegation, ask the user exactly this follow-up question: ${SUBTASK_FANOUT_PARENT_FOLLOWUP}`
32+
export const SUBTASK_FANOUT_XPROFILE_PARENT_MODEL = "openai/gpt-4.1"
33+
export const SUBTASK_FANOUT_XPROFILE_CHILD_MODEL = "openai/gpt-4.1-mini"
34+
export const SUBTASK_FANOUT_XPROFILE_PARENT_FOLLOWUP = "Parent cross-profile fan-out is still isolated?"
35+
export const SUBTASK_FANOUT_XPROFILE_CHILD_RESULT = "Fan-out cross-profile child completed"
36+
const SUBTASK_FANOUT_XPROFILE_CHILD_PROMPT = `${SUBTASK_FANOUT_XPROFILE_CHILD_MARKER}: Complete with the exact result "${SUBTASK_FANOUT_XPROFILE_CHILD_RESULT}".`
37+
export const SUBTASK_FANOUT_XPROFILE_PARENT_PROMPT = `${SUBTASK_FANOUT_XPROFILE_PARENT_MARKER}: Use the new_task tool exactly once. Create an ask-mode subtask with this exact message: "${SUBTASK_FANOUT_XPROFILE_CHILD_PROMPT}" After delegation, ask the user exactly this follow-up question: ${SUBTASK_FANOUT_XPROFILE_PARENT_FOLLOWUP}`
3038

3139
const SUBTASK_INTERRUPT_CHILD_PROMPT = `${SUBTASK_INTERRUPT_CHILD_MARKER}: Ask the user exactly this follow-up question: What is the square root of 81? After the user answers, complete with only the answer.`
3240
export const SUBTASK_INTERRUPT_PARENT_PROMPT = `${SUBTASK_INTERRUPT_PARENT_MARKER}: Use the new_task tool exactly once. Create an ask-mode subtask with this exact message: "${SUBTASK_INTERRUPT_CHILD_PROMPT}" Do not answer directly. When the subtask returns, complete with the exact result "Interrupted parent resumed".`
@@ -198,6 +206,63 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
198206
},
199207
})
200208

209+
mock.addFixture({
210+
match: {
211+
userMessage: new RegExp(SUBTASK_FANOUT_XPROFILE_PARENT_MARKER),
212+
sequenceIndex: 0,
213+
},
214+
response: {
215+
toolCalls: [
216+
{
217+
name: "new_task",
218+
arguments: JSON.stringify({
219+
mode: "ask",
220+
message: SUBTASK_FANOUT_XPROFILE_CHILD_PROMPT,
221+
}),
222+
id: "call_subtasks_fanout_xprofile_parent_new_task_001",
223+
},
224+
],
225+
},
226+
})
227+
228+
mock.addFixture({
229+
match: {
230+
predicate: (req: ChatCompletionRequest) =>
231+
lastUserMessageContains(req, SUBTASK_FANOUT_XPROFILE_CHILD_MARKER) &&
232+
!requestContains(req, [SUBTASK_FANOUT_XPROFILE_PARENT_MARKER]),
233+
},
234+
latency: 15_000,
235+
response: {
236+
toolCalls: [
237+
{
238+
name: "attempt_completion",
239+
arguments: JSON.stringify({ result: SUBTASK_FANOUT_XPROFILE_CHILD_RESULT }),
240+
id: "call_subtasks_fanout_xprofile_child_completion_002",
241+
},
242+
],
243+
},
244+
})
245+
246+
mock.addFixture({
247+
match: {
248+
predicate: (req: ChatCompletionRequest) =>
249+
requestContains(req, [SUBTASK_FANOUT_XPROFILE_PARENT_MARKER, "Delegated to child task"]) &&
250+
!requestContains(req, [SUBTASK_RESULT_INJECTION]),
251+
},
252+
response: {
253+
toolCalls: [
254+
{
255+
name: "ask_followup_question",
256+
arguments: JSON.stringify({
257+
question: SUBTASK_FANOUT_XPROFILE_PARENT_FOLLOWUP,
258+
follow_up: [{ text: "continue" }],
259+
}),
260+
id: "call_subtasks_fanout_xprofile_parent_followup_003",
261+
},
262+
],
263+
},
264+
})
265+
201266
// The parent prompt embeds SUBTASK_FAST_CHILD_MARKER verbatim, so parent-resume turns
202267
// can also match a bare substring check (same collision class as #561). Exclude the
203268
// parent marker so those turns fall through to the parent-resume fixture below.

apps/vscode-e2e/src/suite/subtasks.test.ts

Lines changed: 163 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ import {
2121
SUBTASK_FAST_CHILD_RESULT,
2222
SUBTASK_FANOUT_PARENT_FOLLOWUP,
2323
SUBTASK_FANOUT_PARENT_PROMPT,
24+
SUBTASK_FANOUT_XPROFILE_CHILD_MODEL,
25+
SUBTASK_FANOUT_XPROFILE_CHILD_RESULT,
26+
SUBTASK_FANOUT_XPROFILE_PARENT_FOLLOWUP,
27+
SUBTASK_FANOUT_XPROFILE_PARENT_MODEL,
28+
SUBTASK_FANOUT_XPROFILE_PARENT_PROMPT,
2429
SUBTASK_FAST_PARENT_PROMPT,
2530
SUBTASK_INTERRUPT_CHILD_FOLLOWUP_ANSWER,
2631
SUBTASK_INTERRUPT_PARENT_PROMPT,
@@ -36,6 +41,7 @@ type AimockMessageContent = string | Array<{ type?: string; text?: string }>
3641

3742
type AimockJournalEntry = {
3843
body?: {
44+
model?: string
3945
messages?: Array<{
4046
role?: string
4147
content?: AimockMessageContent
@@ -51,24 +57,55 @@ const messageContentText = (content?: AimockMessageContent) => {
5157
return content?.map((part) => part.text ?? "").join("") ?? ""
5258
}
5359

60+
const requestUserText = (entry: AimockJournalEntry) =>
61+
(entry.body?.messages ?? [])
62+
.filter((message) => message.role === "user")
63+
.map((message) => messageContentText(message.content))
64+
.join("")
65+
5466
const waitForAimockRequestContaining = async (expectedText: string, excludeText?: string) => {
67+
await waitForAimockRequest((entry) => {
68+
const messages = entry.body?.messages
69+
if (!messages) return false
70+
const entryText = messages.map((m) => messageContentText(m.content)).join("")
71+
if (excludeText && entryText.includes(excludeText)) return false
72+
return messages.some(
73+
(message) => message.role === "user" && messageContentText(message.content).includes(expectedText),
74+
)
75+
})
76+
}
77+
78+
const waitForAimockRequest = async (matches: (entry: AimockJournalEntry) => boolean) => {
79+
let latestEntries: AimockJournalEntry[] = []
80+
81+
try {
82+
await waitFor(async () => {
83+
latestEntries = await readAimockJournal()
84+
return latestEntries.some(matches)
85+
})
86+
} catch {
87+
assertAimockRequest(latestEntries, matches)
88+
}
89+
}
90+
91+
const readAimockJournal = async () => {
5592
const aimockUrl = process.env.AIMOCK_URL
5693
assert.ok(aimockUrl, "AIMOCK_URL must be set for aimock journal assertions")
5794

58-
await waitFor(async () => {
59-
const response = await fetch(`${aimockUrl}/__aimock/journal`)
60-
const entries = (await response.json()) as AimockJournalEntry[]
61-
62-
return entries.some((entry) => {
63-
const messages = entry.body?.messages
64-
if (!messages) return false
65-
const entryText = messages.map((m) => messageContentText(m.content)).join("")
66-
if (excludeText && entryText.includes(excludeText)) return false
67-
return messages.some(
68-
(message) => message.role === "user" && messageContentText(message.content).includes(expectedText),
69-
)
70-
})
71-
})
95+
const response = await fetch(`${aimockUrl}/__aimock/journal`)
96+
return (await response.json()) as AimockJournalEntry[]
97+
}
98+
99+
const assertAimockRequest = (entries: AimockJournalEntry[], matches: (entry: AimockJournalEntry) => boolean) => {
100+
if (entries.some(matches)) return
101+
102+
const summary = entries.map((entry) => ({
103+
model: entry.body?.model,
104+
userText: entry.body?.messages
105+
?.filter((message) => message.role === "user")
106+
.map((message) => messageContentText(message.content).slice(0, 180)),
107+
}))
108+
assert.fail(`Expected aimock request was not found. Requests: ${JSON.stringify(summary, null, 2)}`)
72109
}
73110

74111
suite("Roo Code Subtasks", function () {
@@ -189,6 +226,118 @@ suite("Roo Code Subtasks", function () {
189226
}
190227
})
191228

229+
test("fan-out keeps parent API config isolated when child switches to a different saved profile", async () => {
230+
const api = globalThis.api
231+
const asks: Record<string, ClineMessage[]> = {}
232+
233+
const messageHandler = ({ taskId, message }: { taskId: string; message: ClineMessage }) => {
234+
if (message.type === "ask") {
235+
asks[taskId] = asks[taskId] || []
236+
asks[taskId].push(message)
237+
}
238+
}
239+
240+
api.on(RooCodeEventName.Message, messageHandler)
241+
242+
const aimockUrl = process.env.AIMOCK_URL
243+
const parentProfile = {
244+
apiProvider: "openrouter" as const,
245+
openRouterApiKey: "mock-key",
246+
openRouterModelId: SUBTASK_FANOUT_XPROFILE_PARENT_MODEL,
247+
rateLimitSeconds: 0,
248+
...(aimockUrl && { openRouterBaseUrl: `${aimockUrl}/v1` }),
249+
}
250+
const childProfile = {
251+
...parentProfile,
252+
openRouterModelId: SUBTASK_FANOUT_XPROFILE_CHILD_MODEL,
253+
}
254+
const priorModeApiConfigs = api.getConfiguration().modeApiConfigs ?? {}
255+
const parentProfileId = await api.upsertProfile("subtask-fanout-parent-profile", parentProfile, true)
256+
const childProfileId = await api.upsertProfile("subtask-fanout-child-profile", childProfile, false)
257+
await api.setConfiguration({
258+
modeApiConfigs: {
259+
...priorModeApiConfigs,
260+
code: parentProfileId!,
261+
ask: childProfileId!,
262+
},
263+
})
264+
265+
try {
266+
api.setTaskSchedulerMaxConcurrency(2)
267+
268+
const parentTaskId = await api.startNewTask({
269+
configuration: {
270+
mode: "code",
271+
alwaysAllowModeSwitch: true,
272+
alwaysAllowSubtasks: true,
273+
autoApprovalEnabled: true,
274+
enableCheckpoints: false,
275+
},
276+
text: SUBTASK_FANOUT_XPROFILE_PARENT_PROMPT,
277+
})
278+
279+
let childTaskId: string | undefined
280+
await waitFor(() => {
281+
const stack = api.getCurrentTaskStack()
282+
const current = stack.at(-1)
283+
if (current && current !== parentTaskId) {
284+
childTaskId = current
285+
return stack.includes(parentTaskId)
286+
}
287+
return false
288+
})
289+
290+
await waitFor(() =>
291+
(asks[parentTaskId] ?? []).some(
292+
({ ask, text }) => ask === "followup" && text?.includes(SUBTASK_FANOUT_XPROFILE_PARENT_FOLLOWUP),
293+
),
294+
)
295+
296+
assert.ok(
297+
(asks[parentTaskId] ?? []).some(
298+
({ ask, text }) => ask === "followup" && text?.includes(SUBTASK_FANOUT_XPROFILE_PARENT_FOLLOWUP),
299+
),
300+
"Parent should keep running and ask its follow-up using its own profile",
301+
)
302+
303+
const stack = api.getCurrentTaskStack()
304+
assert.ok(stack.includes(parentTaskId), "Fan-out parent should remain in the live task stack")
305+
assert.ok(stack.includes(childTaskId!), "Fan-out child should remain in the live task stack")
306+
assert.strictEqual(stack.at(-1), childTaskId, "Child should remain the focused task while parent runs")
307+
308+
await waitForAimockRequest((entry) => {
309+
const text = requestUserText(entry)
310+
const userMessageCount = (entry.body?.messages ?? []).filter(
311+
(message) => message.role === "user",
312+
).length
313+
return (
314+
entry.body?.model === SUBTASK_FANOUT_XPROFILE_PARENT_MODEL &&
315+
text.includes(SUBTASK_FANOUT_XPROFILE_PARENT_PROMPT) &&
316+
userMessageCount > 1
317+
)
318+
})
319+
await waitForAimockRequest(
320+
(entry) =>
321+
entry.body?.model === SUBTASK_FANOUT_XPROFILE_CHILD_MODEL &&
322+
(entry.body.messages ?? []).some(
323+
(message) =>
324+
message.role === "user" &&
325+
messageContentText(message.content).includes(SUBTASK_FANOUT_XPROFILE_CHILD_RESULT),
326+
),
327+
)
328+
} finally {
329+
api.off(RooCodeEventName.Message, messageHandler)
330+
while (api.getCurrentTaskStack().length > 0) {
331+
await api.clearCurrentTask()
332+
}
333+
api.setTaskSchedulerMaxConcurrency(1)
334+
await waitFor(() => api.getCurrentTaskStack().length === 0).catch(() => {})
335+
await api.setConfiguration({ modeApiConfigs: priorModeApiConfigs })
336+
await api.deleteProfile("subtask-fanout-child-profile").catch(() => {})
337+
await api.deleteProfile("subtask-fanout-parent-profile").catch(() => {})
338+
}
339+
})
340+
192341
// Smoke: child completing normally must resume the parent task.
193342
test("child task returns to parent after normal completion", async () => {
194343
const api = globalThis.api

src/__tests__/helpers/provider-stub.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,14 @@ type ProviderStubFields = {
1313
runDelegationTransition?: unknown
1414
removeClineFromStack?: unknown
1515
evictCurrentTask?: unknown
16+
restoreParentOrReleasePermit?: unknown
1617
}
1718

1819
type PrivateProviderMethods = {
1920
runDelegationTransition: (this: unknown, ...args: unknown[]) => unknown
2021
removeClineFromStack: (this: unknown, ...args: unknown[]) => unknown
2122
evictCurrentTask: (this: unknown, ...args: unknown[]) => unknown
23+
restoreParentOrReleasePermit: (this: unknown, ...args: unknown[]) => unknown
2224
}
2325

2426
/**
@@ -51,5 +53,6 @@ export function makeProviderStub<T extends object>(stub: T): ClineProvider {
5153
s.runDelegationTransition ??= proto.runDelegationTransition.bind(s)
5254
s.removeClineFromStack ??= proto.removeClineFromStack.bind(s)
5355
s.evictCurrentTask ??= proto.evictCurrentTask.bind(s)
56+
s.restoreParentOrReleasePermit ??= proto.restoreParentOrReleasePermit.bind(s)
5457
return s as unknown as ClineProvider
5558
}

src/__tests__/provider-delegation.spec.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,24 @@ import { RooCodeEventName } from "@roo-code/types"
66
import { ClineProvider } from "../core/webview/ClineProvider"
77
import { TaskScheduler } from "../core/task/TaskScheduler"
88

9+
/**
10+
* restoreParentOrReleasePermit is a private prototype method that plain
11+
* object-literal provider stubs don't have unless bound explicitly (same
12+
* reason removeClineFromStack/evictCurrentTask need binding in provider-stub.ts).
13+
*/
14+
function bindRestoreParentOrReleasePermit(provider: ClineProvider): void {
15+
type WithRestore = {
16+
restoreParentOrReleasePermit: (
17+
parentTaskId: string,
18+
fanOut: boolean,
19+
childReservedRelease: (() => void) | undefined,
20+
) => Promise<void>
21+
}
22+
;(provider as unknown as WithRestore).restoreParentOrReleasePermit = (
23+
ClineProvider.prototype as unknown as WithRestore
24+
).restoreParentOrReleasePermit.bind(provider)
25+
}
26+
927
const parentHistoryItem: HistoryItem = {
1028
id: "parent-1",
1129
task: "Parent",
@@ -318,6 +336,7 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => {
318336
recentTasksCache: undefined,
319337
taskHistoryStore,
320338
} as unknown as ClineProvider
339+
bindRestoreParentOrReleasePermit(provider)
321340

322341
await expect(
323342
(ClineProvider.prototype as any).delegateParentAndOpenChild.call(provider, {
@@ -373,6 +392,7 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => {
373392
// still needs evicting, independent of current focus.
374393
taskRegistry: { getById: vi.fn((id: string) => (id === "child-1" ? child : undefined)) },
375394
} as unknown as ClineProvider
395+
bindRestoreParentOrReleasePermit(provider)
376396

377397
await expect(
378398
(ClineProvider.prototype as any).delegateParentAndOpenChild.call(provider, {

0 commit comments

Comments
 (0)