Skip to content

Commit 2f8bf0b

Browse files
F915F915
authored
Merge branch 'main' into bailian
2 parents 719c0a8 + 3df406e commit 2f8bf0b

21 files changed

Lines changed: 1291 additions & 311 deletions

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

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
import { LLMock } from "@copilotkit/aimock"
2+
import type { ChatCompletionRequest } from "@copilotkit/aimock"
3+
4+
import { toolResultContains } from "./tool-result"
5+
6+
const SUBTASK_PARENT_MARKER = "SUBTASK_PARENT_CANCELLATION_SMOKE"
7+
const SUBTASK_CHILD_MARKER = "SUBTASK_CHILD_CALCULATOR_SMOKE"
8+
9+
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.`
10+
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.`
11+
export const SUBTASK_CHILD_FOLLOWUP_ANSWER = "9"
12+
13+
const requestContains = (req: ChatCompletionRequest, expected: string[]) => {
14+
const rawRequest = JSON.stringify(req)
15+
return expected.every((text) => rawRequest.includes(text))
16+
}
17+
18+
const completionAfterAnswer = (followupId: string, completionId: string) => ({
19+
match: {
20+
predicate: (req: ChatCompletionRequest) =>
21+
// Preferred: structured tool-result message carries the followup answer.
22+
toolResultContains(req, followupId, [SUBTASK_CHILD_FOLLOWUP_ANSWER]) ||
23+
// Fallback 1: answer present alongside the tool-call ID but not in a role:tool message.
24+
requestContains(req, [followupId, SUBTASK_CHILD_FOLLOWUP_ANSWER]) ||
25+
// Fallback 2: answer arrives as a bare user message after task resume (no tool-call ID context).
26+
requestContains(req, [
27+
SUBTASK_CHILD_MARKER,
28+
`<user_message>\\n${SUBTASK_CHILD_FOLLOWUP_ANSWER}\\n</user_message>`,
29+
]),
30+
},
31+
response: {
32+
toolCalls: [
33+
{
34+
name: "attempt_completion",
35+
arguments: JSON.stringify({ result: "9" }),
36+
id: completionId,
37+
},
38+
],
39+
},
40+
})
41+
42+
export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
43+
mock.addFixture({
44+
match: {
45+
userMessage: new RegExp(SUBTASK_PARENT_MARKER),
46+
},
47+
response: {
48+
toolCalls: [
49+
{
50+
name: "new_task",
51+
arguments: JSON.stringify({
52+
mode: "ask",
53+
message: SUBTASK_CHILD_PROMPT,
54+
}),
55+
id: "call_subtasks_parent_new_task_001",
56+
},
57+
],
58+
},
59+
})
60+
61+
mock.addFixture({
62+
match: {
63+
userMessage: new RegExp(SUBTASK_CHILD_MARKER),
64+
},
65+
response: {
66+
toolCalls: [
67+
{
68+
name: "ask_followup_question",
69+
arguments: JSON.stringify({
70+
question: "What is the square root of 81?",
71+
follow_up: [{ text: SUBTASK_CHILD_FOLLOWUP_ANSWER }],
72+
}),
73+
id: "call_subtasks_child_followup_001",
74+
},
75+
],
76+
},
77+
})
78+
79+
mock.addFixture(completionAfterAnswer("call_subtasks_child_followup_001", "call_subtasks_child_completion_002"))
80+
81+
mock.addFixture({
82+
match: {
83+
toolCallId: "call_subtasks_parent_new_task_001",
84+
},
85+
response: {
86+
toolCalls: [
87+
{
88+
name: "attempt_completion",
89+
arguments: JSON.stringify({ result: "Parent task resumed" }),
90+
id: "call_subtasks_parent_completion_003",
91+
},
92+
],
93+
},
94+
})
95+
}

‎apps/vscode-e2e/src/runTest.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { addExecuteCommandResultFixtures } from "./fixtures/execute-command"
1010
import { addListFilesResultFixtures } from "./fixtures/list-files"
1111
import { addReadFileResultFixtures } from "./fixtures/read-file"
1212
import { addSearchFilesResultFixtures } from "./fixtures/search-files"
13+
import { addSubtaskFixtures } from "./fixtures/subtasks"
1314
import { addUseMcpToolResultFixtures } from "./fixtures/use-mcp-tool"
1415
import { addWriteToFileResultFixtures } from "./fixtures/write-to-file"
1516

@@ -110,6 +111,7 @@ async function main() {
110111
addListFilesResultFixtures(mock)
111112
addReadFileResultFixtures(mock)
112113
addSearchFilesResultFixtures(mock)
114+
addSubtaskFixtures(mock)
113115
addUseMcpToolResultFixtures(mock)
114116
addWriteToFileResultFixtures(mock)
115117

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

Lines changed: 172 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -2,73 +2,191 @@ import * as assert from "assert"
22

33
import { RooCodeEventName, type ClineMessage } from "@roo-code/types"
44

5-
import { sleep, waitFor, waitUntilCompleted } from "./utils"
5+
import { setDefaultSuiteTimeout } from "./test-utils"
6+
import { waitFor, waitUntilCompleted } from "./utils"
7+
import { SUBTASK_CHILD_FOLLOWUP_ANSWER, SUBTASK_PARENT_PROMPT } from "../fixtures/subtasks"
68

7-
suite.skip("Roo Code Subtasks", () => {
8-
test("Should handle subtask cancellation and resumption correctly", async () => {
9+
suite("Roo Code Subtasks", function () {
10+
setDefaultSuiteTimeout(this)
11+
12+
// Race mitigation: skipDelegationRepair prevents removeClineFromStack from
13+
// auto-resuming the parent when the child is cancelled (Race 2).
14+
test("parent stays paused after subtask cancellation", async () => {
915
const api = globalThis.api
16+
const asks: Record<string, ClineMessage[]> = {}
17+
const messages: Record<string, ClineMessage[]> = {}
18+
19+
const messageHandler = ({ taskId, message }: { taskId: string; message: ClineMessage }) => {
20+
if (message.type === "ask") {
21+
asks[taskId] = asks[taskId] || []
22+
asks[taskId].push(message)
23+
}
24+
if (message.type === "say" && message.partial === false) {
25+
messages[taskId] = messages[taskId] || []
26+
messages[taskId].push(message)
27+
}
28+
}
29+
30+
api.on(RooCodeEventName.Message, messageHandler)
31+
32+
try {
33+
const parentTaskId = await api.startNewTask({
34+
configuration: {
35+
mode: "ask",
36+
alwaysAllowModeSwitch: true,
37+
alwaysAllowSubtasks: true,
38+
autoApprovalEnabled: true,
39+
enableCheckpoints: false,
40+
},
41+
text: SUBTASK_PARENT_PROMPT,
42+
})
43+
44+
let spawnedTaskId: string | undefined
45+
await waitFor(() => {
46+
const stack = api.getCurrentTaskStack()
47+
const current = stack[stack.length - 1]
48+
if (current && current !== parentTaskId) {
49+
spawnedTaskId = current
50+
return true
51+
}
52+
return false
53+
})
54+
55+
await waitFor(
56+
() => asks[spawnedTaskId!]?.some(({ type, ask }) => type === "ask" && ask === "followup") ?? false,
57+
)
58+
59+
await api.cancelCurrentTask()
60+
61+
assert.ok(
62+
messages[parentTaskId]?.find(({ type, text }) => type === "say" && text === "Parent task resumed") ===
63+
undefined,
64+
"Parent task should not have resumed after subtask cancellation",
65+
)
1066

67+
await waitFor(() => api.getCurrentTaskStack().at(-1) === spawnedTaskId)
68+
await waitFor(
69+
() => asks[spawnedTaskId!]?.some(({ type, ask }) => type === "ask" && ask === "resume_task") ?? false,
70+
)
71+
72+
await api.clearCurrentTask()
73+
// The parent task is still in the stack; drain it so it doesn't leak into the next test.
74+
await api.clearCurrentTask()
75+
await waitFor(() => api.getCurrentTaskStack().length === 0)
76+
} finally {
77+
api.off(RooCodeEventName.Message, messageHandler)
78+
}
79+
})
80+
81+
// Race mitigation: runDelegationTransition lock + cancelledDelegationChildIds guard
82+
// ensures cancelTask() wins over a concurrent reopenParentFromDelegation() (Race 3).
83+
test("cancelled child completes in-place and does not reopen parent", async () => {
84+
const api = globalThis.api
85+
const asks: Record<string, ClineMessage[]> = {}
1186
const messages: Record<string, ClineMessage[]> = {}
1287

13-
api.on(RooCodeEventName.Message, ({ taskId, message }) => {
88+
const messageHandler = ({ taskId, message }: { taskId: string; message: ClineMessage }) => {
89+
if (message.type === "ask") {
90+
asks[taskId] = asks[taskId] || []
91+
asks[taskId].push(message)
92+
}
1493
if (message.type === "say" && message.partial === false) {
1594
messages[taskId] = messages[taskId] || []
1695
messages[taskId].push(message)
1796
}
18-
})
19-
20-
const childPrompt = "You are a calculator. Respond only with numbers. What is the square root of 9?"
21-
22-
// Start a parent task that will create a subtask.
23-
const parentTaskId = await api.startNewTask({
24-
configuration: {
25-
mode: "ask",
26-
alwaysAllowModeSwitch: true,
27-
alwaysAllowSubtasks: true,
28-
autoApprovalEnabled: true,
29-
enableCheckpoints: false,
30-
},
31-
text:
32-
"You are the parent task. " +
33-
`Create a subtask by using the new_task tool with the message '${childPrompt}'.` +
34-
"After creating the subtask, wait for it to complete and then respond 'Parent task resumed'.",
35-
})
36-
37-
let spawnedTaskId: string | undefined = undefined
38-
39-
// Wait for the subtask to be spawned and then cancel it.
40-
api.on(RooCodeEventName.TaskSpawned, (_, childTaskId) => (spawnedTaskId = childTaskId))
41-
await waitFor(() => !!spawnedTaskId)
42-
await sleep(1_000) // Give the task a chance to start and populate the history.
43-
await api.cancelCurrentTask()
44-
45-
// Wait a bit to ensure any task resumption would have happened.
46-
await sleep(2_000)
47-
48-
// The parent task should not have resumed yet, so we shouldn't see
49-
// "Parent task resumed".
50-
assert.ok(
51-
messages[parentTaskId]?.find(({ type, text }) => type === "say" && text === "Parent task resumed") ===
52-
undefined,
53-
"Parent task should not have resumed after subtask cancellation",
54-
)
97+
}
98+
99+
const findCompletionText = (taskId: string) =>
100+
messages[taskId]
101+
?.filter(
102+
(message) =>
103+
message.type === "say" && (message.say === "completion_result" || message.say === "text"),
104+
)
105+
.map((message) => message.text?.trim())
106+
.find((text): text is string => !!text)
107+
108+
const findErrorText = (taskId: string) =>
109+
messages[taskId]
110+
?.filter((message) => message.type === "say" && message.say === "error")
111+
.map((message) => message.text?.trim())
112+
.find((text): text is string => !!text)
113+
114+
api.on(RooCodeEventName.Message, messageHandler)
115+
116+
try {
117+
const parentTaskId = await api.startNewTask({
118+
configuration: {
119+
mode: "ask",
120+
alwaysAllowModeSwitch: true,
121+
alwaysAllowSubtasks: true,
122+
autoApprovalEnabled: true,
123+
enableCheckpoints: false,
124+
},
125+
text: SUBTASK_PARENT_PROMPT,
126+
})
127+
128+
let spawnedTaskId: string | undefined
129+
await waitFor(() => {
130+
const stack = api.getCurrentTaskStack()
131+
const current = stack[stack.length - 1]
132+
if (current && current !== parentTaskId) {
133+
spawnedTaskId = current
134+
return true
135+
}
136+
return false
137+
})
138+
139+
await waitFor(
140+
() => asks[spawnedTaskId!]?.some(({ type, ask }) => type === "ask" && ask === "followup") ?? false,
141+
)
142+
143+
const cancelledChildTaskId = spawnedTaskId!
144+
await api.cancelCurrentTask()
55145

56-
// Start a new task with the same message as the subtask.
57-
const anotherTaskId = await api.startNewTask({ text: childPrompt })
58-
await waitUntilCompleted({ api, taskId: anotherTaskId })
146+
await waitFor(() => api.getCurrentTaskStack().at(-1) === cancelledChildTaskId)
147+
await waitFor(
148+
() =>
149+
asks[cancelledChildTaskId]?.some(({ type, ask }) => type === "ask" && ask === "resume_task") ??
150+
false,
151+
)
59152

60-
// Wait a bit to ensure any task resumption would have happened.
61-
await sleep(2_000)
153+
const resumedChildTaskId = await waitUntilCompleted({
154+
api,
155+
start: async () => {
156+
await api.sendMessage(SUBTASK_CHILD_FOLLOWUP_ANSWER)
157+
return cancelledChildTaskId
158+
},
159+
})
62160

63-
// The parent task should still not have resumed.
64-
assert.ok(
65-
messages[parentTaskId]?.find(({ type, text }) => type === "say" && text === "Parent task resumed") ===
161+
assert.strictEqual(
162+
resumedChildTaskId,
163+
cancelledChildTaskId,
164+
"Cancelled child task should be resumed in place",
165+
)
166+
assert.strictEqual(
167+
findErrorText(resumedChildTaskId),
66168
undefined,
67-
"Parent task should not have resumed after subtask cancellation",
68-
)
169+
"Resumed child task should not emit an error",
170+
)
171+
assert.strictEqual(
172+
findCompletionText(resumedChildTaskId),
173+
"9",
174+
"Resumed child task should complete with `9`",
175+
)
176+
assert.strictEqual(
177+
api.getCurrentTaskStack().at(-1),
178+
cancelledChildTaskId,
179+
"Cancelled child task should remain the active completed task",
180+
)
181+
assert.ok(
182+
messages[parentTaskId]?.find(({ type, text }) => type === "say" && text === "Parent task resumed") ===
183+
undefined,
184+
"Parent task should not have resumed after the cancelled child completed",
185+
)
69186

70-
// Clean up - cancel all tasks.
71-
await api.clearCurrentTask()
72-
await waitUntilCompleted({ api, taskId: parentTaskId })
187+
await api.clearCurrentTask()
188+
} finally {
189+
api.off(RooCodeEventName.Message, messageHandler)
190+
}
73191
})
74192
})

‎apps/vscode-e2e/src/suite/tools/apply-diff.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ suite("Roo Code apply_diff Tool", function () {
128128

129129
suiteTeardown(async () => {
130130
try {
131-
await globalThis.api.cancelCurrentTask()
131+
await globalThis.api.clearCurrentTask()
132132
} catch {
133133
// Task might not be running
134134
}
@@ -147,7 +147,7 @@ suite("Roo Code apply_diff Tool", function () {
147147

148148
setup(async () => {
149149
try {
150-
await globalThis.api.cancelCurrentTask()
150+
await globalThis.api.clearCurrentTask()
151151
} catch {
152152
// Task might not be running
153153
}
@@ -164,7 +164,7 @@ suite("Roo Code apply_diff Tool", function () {
164164

165165
teardown(async () => {
166166
try {
167-
await globalThis.api.cancelCurrentTask()
167+
await globalThis.api.clearCurrentTask()
168168
} catch {
169169
// Task might not be running
170170
}

0 commit comments

Comments
 (0)