Skip to content

Commit 7516e0d

Browse files
edelaunaedelauna
committed
test(e2e): simplyfying e2e tests
1 parent d257774 commit 7516e0d

5 files changed

Lines changed: 96 additions & 209 deletions

File tree

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { toolResultContains } from "./tool-result"
66
const SUBTASK_PARENT_MARKER = "SUBTASK_PARENT_CANCELLATION_SMOKE"
77
const SUBTASK_CHILD_MARKER = "SUBTASK_CHILD_CALCULATOR_SMOKE"
88

9-
export 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.`
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.`
1010
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.`
1111
export const SUBTASK_CHILD_FOLLOWUP_ANSWER = "9"
1212

@@ -18,8 +18,11 @@ const requestContains = (req: ChatCompletionRequest, expected: string[]) => {
1818
const completionAfterAnswer = (followupId: string, completionId: string) => ({
1919
match: {
2020
predicate: (req: ChatCompletionRequest) =>
21+
// Preferred: structured tool-result message carries the followup answer.
2122
toolResultContains(req, followupId, [SUBTASK_CHILD_FOLLOWUP_ANSWER]) ||
23+
// Fallback 1: answer present alongside the tool-call ID but not in a role:tool message.
2224
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).
2326
requestContains(req, [
2427
SUBTASK_CHILD_MARKER,
2528
`<user_message>\\n${SUBTASK_CHILD_FOLLOWUP_ANSWER}\\n</user_message>`,

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

Lines changed: 80 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -9,25 +9,87 @@ import { SUBTASK_CHILD_FOLLOWUP_ANSWER, SUBTASK_PARENT_PROMPT } from "../fixture
99
suite("Roo Code Subtasks", function () {
1010
setDefaultSuiteTimeout(this)
1111

12-
test("Should keep parent paused after subtask cancellation", async () => {
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 () => {
1315
const api = globalThis.api
1416
const asks: Record<string, ClineMessage[]> = {}
1517
const messages: Record<string, ClineMessage[]> = {}
16-
const waitForStage = async (label: string, condition: Parameters<typeof waitFor>[0]) => {
17-
try {
18-
await waitFor(condition)
19-
} catch (error) {
20-
const message = error instanceof Error ? error.message : String(error)
21-
throw new Error(`${label}: ${message}`)
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)
2227
}
2328
}
2429

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+
)
66+
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[]> = {}
86+
const messages: Record<string, ClineMessage[]> = {}
87+
2588
const messageHandler = ({ taskId, message }: { taskId: string; message: ClineMessage }) => {
2689
if (message.type === "ask") {
2790
asks[taskId] = asks[taskId] || []
2891
asks[taskId].push(message)
2992
}
30-
3193
if (message.type === "say" && message.partial === false) {
3294
messages[taskId] = messages[taskId] || []
3395
messages[taskId].push(message)
@@ -64,35 +126,25 @@ suite("Roo Code Subtasks", function () {
64126
})
65127

66128
let spawnedTaskId: string | undefined
67-
await waitForStage("wait for spawned subtask", () => {
68-
const currentTaskStack = api.getCurrentTaskStack()
69-
const currentTaskId = currentTaskStack[currentTaskStack.length - 1]
70-
if (currentTaskId && currentTaskId !== parentTaskId) {
71-
spawnedTaskId = currentTaskId
129+
await waitFor(() => {
130+
const stack = api.getCurrentTaskStack()
131+
const current = stack[stack.length - 1]
132+
if (current && current !== parentTaskId) {
133+
spawnedTaskId = current
72134
return true
73135
}
74136
return false
75137
})
76-
await waitForStage(
77-
"wait for delegated child followup ask",
138+
139+
await waitFor(
78140
() => asks[spawnedTaskId!]?.some(({ type, ask }) => type === "ask" && ask === "followup") ?? false,
79141
)
80-
const cancelledChildTaskId = spawnedTaskId!
81142

143+
const cancelledChildTaskId = spawnedTaskId!
82144
await api.cancelCurrentTask()
83145

84-
assert.ok(
85-
messages[parentTaskId]?.find(({ type, text }) => type === "say" && text === "Parent task resumed") ===
86-
undefined,
87-
"Parent task should not have resumed after subtask cancellation",
88-
)
89-
90-
await waitForStage(
91-
"wait for cancelled child task to remain active",
92-
() => api.getCurrentTaskStack().at(-1) === cancelledChildTaskId,
93-
)
94-
await waitForStage(
95-
"wait for cancelled child resume ask",
146+
await waitFor(() => api.getCurrentTaskStack().at(-1) === cancelledChildTaskId)
147+
await waitFor(
96148
() =>
97149
asks[cancelledChildTaskId]?.some(({ type, ask }) => type === "ask" && ask === "resume_task") ??
98150
false,
@@ -126,7 +178,6 @@ suite("Roo Code Subtasks", function () {
126178
cancelledChildTaskId,
127179
"Cancelled child task should remain the active completed task",
128180
)
129-
130181
assert.ok(
131182
messages[parentTaskId]?.find(({ type, text }) => type === "say" && text === "Parent task resumed") ===
132183
undefined,

‎src/__tests__/provider-delegation.spec.ts‎

Lines changed: 0 additions & 126 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,6 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => {
101101
const parentTask = {
102102
taskId: "parent-1",
103103
emit: vi.fn(),
104-
getTaskMode: vi.fn().mockResolvedValue("architect"),
105104
} as any
106105
const childStart = vi.fn(() => callOrder.push("child.start"))
107106

@@ -146,129 +145,4 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => {
146145
// Verify ordering: createTask → updateTaskHistory → child.start
147146
expect(callOrder).toEqual(["createTask", "updateTaskHistory", "child.start"])
148147
})
149-
150-
it("removes the paused child when parent delegation metadata cannot be persisted", async () => {
151-
const parentTask = {
152-
taskId: "parent-1",
153-
emit: vi.fn(),
154-
getTaskMode: vi.fn().mockResolvedValue("architect"),
155-
} as any
156-
const childStart = vi.fn()
157-
const persistError = new Error("history write failed")
158-
159-
const providerEmit = vi.fn()
160-
const updateTaskHistory = vi.fn().mockRejectedValue(persistError)
161-
const removeClineFromStack = vi.fn().mockResolvedValue(undefined)
162-
const deleteTaskFromState = vi.fn().mockResolvedValue(undefined)
163-
const createTask = vi.fn().mockResolvedValue({ taskId: "child-1", start: childStart })
164-
const createTaskWithHistoryItem = vi.fn().mockResolvedValue(undefined)
165-
const handleModeSwitch = vi.fn().mockResolvedValue(undefined)
166-
const parentHistory = {
167-
id: "parent-1",
168-
task: "Parent",
169-
tokensIn: 0,
170-
tokensOut: 0,
171-
totalCost: 0,
172-
childIds: [],
173-
}
174-
const getTaskWithId = vi.fn().mockResolvedValue({
175-
historyItem: parentHistory,
176-
})
177-
178-
const provider = {
179-
emit: providerEmit,
180-
getCurrentTask: vi.fn(() => parentTask),
181-
removeClineFromStack,
182-
createTask,
183-
getTaskWithId,
184-
updateTaskHistory,
185-
deleteTaskFromState,
186-
createTaskWithHistoryItem,
187-
handleModeSwitch,
188-
log: vi.fn(),
189-
} as unknown as ClineProvider
190-
191-
await expect(
192-
(ClineProvider.prototype as any).delegateParentAndOpenChild.call(provider, {
193-
parentTaskId: "parent-1",
194-
message: "Do something",
195-
initialTodos: [],
196-
mode: "code",
197-
}),
198-
).rejects.toThrow("history write failed")
199-
200-
expect(createTask).toHaveBeenCalledWith("Do something", undefined, parentTask, {
201-
initialTodos: [],
202-
initialStatus: "active",
203-
startTask: false,
204-
})
205-
expect(childStart).not.toHaveBeenCalled()
206-
expect(removeClineFromStack).toHaveBeenNthCalledWith(1, { skipDelegationRepair: true })
207-
expect(removeClineFromStack).toHaveBeenNthCalledWith(2, { skipDelegationRepair: true })
208-
expect(deleteTaskFromState).toHaveBeenCalledWith("child-1")
209-
expect(createTaskWithHistoryItem).toHaveBeenCalledWith(
210-
{ ...parentHistory, mode: "architect" },
211-
{ startTask: false },
212-
)
213-
expect(providerEmit).not.toHaveBeenCalledWith(RooCodeEventName.TaskDelegated, "parent-1", "child-1")
214-
})
215-
216-
it("logs rollback cleanup failures while preserving the parent metadata error", async () => {
217-
const parentTask = {
218-
taskId: "parent-1",
219-
emit: vi.fn(),
220-
getTaskMode: vi.fn().mockRejectedValue(new Error("mode unavailable")),
221-
} as any
222-
const childStart = vi.fn()
223-
const persistError = new Error("history write failed")
224-
const log = vi.fn()
225-
226-
const removeClineFromStack = vi
227-
.fn()
228-
.mockResolvedValueOnce(undefined)
229-
.mockRejectedValueOnce(new Error("child stack cleanup failed"))
230-
const deleteTaskFromState = vi.fn().mockRejectedValue(new Error("child history cleanup failed"))
231-
const createTaskWithHistoryItem = vi.fn().mockRejectedValue(new Error("parent rehydrate failed"))
232-
const parentHistory = {
233-
id: "parent-1",
234-
task: "Parent",
235-
tokensIn: 0,
236-
tokensOut: 0,
237-
totalCost: 0,
238-
childIds: [],
239-
mode: "ask",
240-
}
241-
242-
const provider = {
243-
emit: vi.fn(),
244-
getCurrentTask: vi.fn(() => parentTask),
245-
removeClineFromStack,
246-
createTask: vi.fn().mockResolvedValue({ taskId: "child-1", start: childStart }),
247-
getTaskWithId: vi.fn().mockResolvedValue({ historyItem: parentHistory }),
248-
updateTaskHistory: vi.fn().mockRejectedValue(persistError),
249-
deleteTaskFromState,
250-
createTaskWithHistoryItem,
251-
handleModeSwitch: vi.fn().mockResolvedValue(undefined),
252-
log,
253-
} as unknown as ClineProvider
254-
255-
await expect(
256-
(ClineProvider.prototype as any).delegateParentAndOpenChild.call(provider, {
257-
parentTaskId: "parent-1",
258-
message: "Do something",
259-
initialTodos: [],
260-
mode: "code",
261-
}),
262-
).rejects.toThrow("history write failed")
263-
264-
expect(childStart).not.toHaveBeenCalled()
265-
expect(deleteTaskFromState).toHaveBeenCalledWith("child-1")
266-
expect(createTaskWithHistoryItem).toHaveBeenCalledWith(parentHistory, { startTask: false })
267-
expect(log).toHaveBeenCalledWith(expect.stringContaining("Failed to capture parent mode"))
268-
expect(log).toHaveBeenCalledWith(
269-
expect.stringContaining("Failed to remove child child-1 after parent metadata"),
270-
)
271-
expect(log).toHaveBeenCalledWith(expect.stringContaining("Failed to remove child child-1 history"))
272-
expect(log).toHaveBeenCalledWith(expect.stringContaining("Failed to rehydrate parent parent-1"))
273-
})
274148
})

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ import { sanitizeToolUseId } from "../../utils/tool-id"
5959
*/
6060

6161
export async function presentAssistantMessage(cline: Task) {
62+
// Silent return is safe here: the lock is not yet held, and the streaming
63+
// loop's own abort check fires before pWaitFor(userMessageContentReady).
6264
if (cline.abort) {
6365
return
6466
}

0 commit comments

Comments
 (0)