Skip to content

Commit 116acfd

Browse files
[Fix] Flaky mocked e2e subtasks test when parent resume is served the child fixture (#1002)
* test(e2e-mock): fix subtasks fixture collision between child and parent-resume fixtures (#1001) The fast-child and SUBTASK_CHILD fixtures matched a bare marker that the parent prompt embeds verbatim, so parent-resume turns could be served the child fixture (mirrors #561, which fixed the same class for cross-profile fixtures only). Parent-resume fixtures also guarded on the literal new_task tool-call id, which validateAndFixToolResultIds can rewrite on resume. - Add parent-marker exclusion (with aimock's last-user-message scoping) to the fast-child and SUBTASK_CHILD fixtures - Guard parent-resume fixtures on the injected subtask result content instead of the fragile tool-call id * test(e2e-mock): address PR review — injection-guard all parent fixtures, link fixture format contract - Regular parent turn-1 fixture: exclude resume turns via SUBTASK_RESULT_INJECTION instead of relying on registration order (sequenceIndex is not viable here — the fixture is shared by 4 tests) - Fast and api-hang parent-resume guards: match SUBTASK_RESULT_INJECTION instead of the child result text, which is embedded verbatim in the parent prompts and could match the parent's initial request on retry - Add unit-test assertion pinning reopenParentFromDelegation's injected tool_result format to the 'completed.\n\nResult:' contract the e2e fixtures match on - lastUserMessageContains: mirror aimock's getTextContent (text parts only) instead of JSON.stringify for non-string content - Extract SUBTASK_FAST_CHILD_RESULT constant - Replace tautological find()/assert pairs in the api-hang test - Move the no-parent-resume assertion in the cancellation test behind the async settle gates and also assert no completion_result --------- Co-authored-by: Roomote <roomote@roomote.dev>
1 parent d1f3999 commit 116acfd

3 files changed

Lines changed: 77 additions & 20 deletions

File tree

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

Lines changed: 54 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ const SUBTASK_XPROFILE_DIFFERENT_CHILD_MARKER = "SUBTASK_CHILD_DIFFERENT_PROFILE
1818
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.`
1919
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.`
2020
export const SUBTASK_CHILD_FOLLOWUP_ANSWER = "9"
21-
const SUBTASK_FAST_CHILD_PROMPT = `${SUBTASK_FAST_CHILD_MARKER}: Complete immediately with the exact result "Fast child completed".`
21+
export const SUBTASK_FAST_CHILD_RESULT = "Fast child completed"
22+
const SUBTASK_FAST_CHILD_PROMPT = `${SUBTASK_FAST_CHILD_MARKER}: Complete immediately with the exact result "${SUBTASK_FAST_CHILD_RESULT}".`
2223
export const SUBTASK_FAST_PARENT_PROMPT = `${SUBTASK_FAST_PARENT_MARKER}: Use the new_task tool exactly once. Create an ask-mode subtask with this exact message: "${SUBTASK_FAST_CHILD_PROMPT}" Do not answer directly.`
2324

2425
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.`
@@ -54,6 +55,31 @@ const requestContains = (req: ChatCompletionRequest, expected: string[]) => {
5455
return expected.every((text) => rawRequest.includes(text))
5556
}
5657

58+
// aimock's `userMessage` matcher only inspects the LAST user message and joins only the
59+
// `type: "text"` content parts (see getTextContent in aimock's router). Fixtures that need
60+
// whole-request exclusions must replicate that scoping inside a predicate so they keep the
61+
// same matching semantics as the bare-regex fixtures they replace.
62+
const lastUserMessageContains = (req: ChatCompletionRequest, text: string) => {
63+
const userMessages = req.messages?.filter((message) => message.role === "user") ?? []
64+
const last = userMessages.at(-1)
65+
if (!last) return false
66+
const content =
67+
typeof last.content === "string"
68+
? last.content
69+
: (last.content ?? [])
70+
.filter((part): part is { type: "text"; text: string } => part?.type === "text")
71+
.map((part) => part.text)
72+
.join("")
73+
return content.includes(text)
74+
}
75+
76+
// reopenParentFromDelegation injects the child result into the resumed parent's history as
77+
// `Subtask <childId> completed.\n\nResult:\n<summary>`. Matching on this injected prefix (in
78+
// its JSON-serialized form) keeps parent-resume fixtures robust when
79+
// validateAndFixToolResultIds rewrites tool-use ids on resume — matching on the new_task
80+
// tool-call id directly proved flaky (the id can be rewritten, the fixture then misses, and
81+
// a looser child fixture wins and serves the child's response to the parent).
82+
const SUBTASK_RESULT_INJECTION = "completed.\\n\\nResult:"
5783
const completionAfterAnswer = (followupId: string, completionId: string) => ({
5884
match: {
5985
predicate: (req: ChatCompletionRequest) =>
@@ -100,25 +126,33 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
100126
},
101127
})
102128

129+
// The parent prompt embeds SUBTASK_FAST_CHILD_MARKER verbatim, so parent-resume turns
130+
// can also match a bare substring check (same collision class as #561). Exclude the
131+
// parent marker so those turns fall through to the parent-resume fixture below.
103132
mock.addFixture({
104133
match: {
105-
userMessage: new RegExp(SUBTASK_FAST_CHILD_MARKER),
134+
predicate: (req: ChatCompletionRequest) =>
135+
lastUserMessageContains(req, SUBTASK_FAST_CHILD_MARKER) &&
136+
!requestContains(req, [SUBTASK_FAST_PARENT_MARKER]),
106137
},
107138
response: {
108139
toolCalls: [
109140
{
110141
name: "attempt_completion",
111-
arguments: JSON.stringify({ result: "Fast child completed" }),
142+
arguments: JSON.stringify({ result: SUBTASK_FAST_CHILD_RESULT }),
112143
id: "call_subtasks_fast_child_completion_002",
113144
},
114145
],
115146
},
116147
})
117148

149+
// Guard on SUBTASK_RESULT_INJECTION (not the child result text): the child result is
150+
// embedded verbatim in SUBTASK_FAST_PARENT_PROMPT, so it cannot distinguish the parent's
151+
// initial request from its resume turn.
118152
mock.addFixture({
119153
match: {
120154
predicate: (req: ChatCompletionRequest) =>
121-
requestContains(req, [SUBTASK_FAST_PARENT_MARKER, "call_subtasks_fast_parent_new_task_001"]),
155+
requestContains(req, [SUBTASK_FAST_PARENT_MARKER, SUBTASK_RESULT_INJECTION]),
122156
},
123157
response: {
124158
toolCalls: [
@@ -131,9 +165,15 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
131165
},
132166
})
133167

168+
// This fixture is shared by several tests, so sequenceIndex cannot guard it. Exclude
169+
// resume turns via the injected tool_result prefix instead: when the tool_result is
170+
// serialized as role:"tool", the original parent prompt is the last user message again
171+
// and would otherwise re-serve new_task on the parent's resume turn.
134172
mock.addFixture({
135173
match: {
136-
userMessage: new RegExp(SUBTASK_PARENT_MARKER),
174+
predicate: (req: ChatCompletionRequest) =>
175+
lastUserMessageContains(req, SUBTASK_PARENT_MARKER) &&
176+
!requestContains(req, [SUBTASK_RESULT_INJECTION]),
137177
},
138178
response: {
139179
toolCalls: [
@@ -149,9 +189,12 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
149189
},
150190
})
151191

192+
// Same collision guard as the fast-child fixture above: SUBTASK_PARENT_PROMPT embeds
193+
// SUBTASK_CHILD_MARKER verbatim, so parent-resume turns must not match this fixture.
152194
mock.addFixture({
153195
match: {
154-
userMessage: new RegExp(SUBTASK_CHILD_MARKER),
196+
predicate: (req: ChatCompletionRequest) =>
197+
lastUserMessageContains(req, SUBTASK_CHILD_MARKER) && !requestContains(req, [SUBTASK_PARENT_MARKER]),
155198
},
156199
response: {
157200
toolCalls: [
@@ -172,7 +215,7 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
172215
mock.addFixture({
173216
match: {
174217
predicate: (req: ChatCompletionRequest) =>
175-
requestContains(req, [SUBTASK_PARENT_MARKER, "call_subtasks_parent_new_task_001"]),
218+
requestContains(req, [SUBTASK_PARENT_MARKER, SUBTASK_RESULT_INJECTION]),
176219
},
177220
response: {
178221
toolCalls: [
@@ -238,14 +281,12 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
238281
},
239282
})
240283

284+
// Same as the fast parent-resume fixture: the child result is embedded verbatim in
285+
// SUBTASK_API_HANG_PARENT_PROMPT, so guard on the injected tool_result prefix instead.
241286
mock.addFixture({
242287
match: {
243288
predicate: (req: ChatCompletionRequest) =>
244-
requestContains(req, [
245-
SUBTASK_API_HANG_PARENT_MARKER,
246-
"call_api_hang_parent_new_task_001",
247-
SUBTASK_API_HANG_CHILD_RESULT,
248-
]),
289+
requestContains(req, [SUBTASK_API_HANG_PARENT_MARKER, SUBTASK_RESULT_INJECTION]),
249290
},
250291
response: {
251292
toolCalls: [
@@ -433,7 +474,7 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
433474
mock.addFixture({
434475
match: {
435476
predicate: (req: ChatCompletionRequest) =>
436-
requestContains(req, [SUBTASK_INTERRUPT_PARENT_MARKER, "call_interrupt_parent_new_task_001"]),
477+
requestContains(req, [SUBTASK_INTERRUPT_PARENT_MARKER, SUBTASK_RESULT_INJECTION]),
437478
},
438479
response: {
439480
toolCalls: [

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

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
SUBTASK_API_HANG_PARENT_RESULT,
1515
SUBTASK_API_HANG_RESUME_MESSAGE,
1616
SUBTASK_CHILD_FOLLOWUP_ANSWER,
17+
SUBTASK_FAST_CHILD_RESULT,
1718
SUBTASK_FAST_PARENT_PROMPT,
1819
SUBTASK_INTERRUPT_CHILD_FOLLOWUP_ANSWER,
1920
SUBTASK_INTERRUPT_PARENT_PROMPT,
@@ -101,7 +102,8 @@ suite("Roo Code Subtasks", function () {
101102
([taskId, messages]) =>
102103
taskId !== parentTaskId &&
103104
messages.some(
104-
({ say, text }) => say === "completion_result" && text?.trim() === "Fast child completed",
105+
({ say, text }) =>
106+
say === "completion_result" && text?.trim() === SUBTASK_FAST_CHILD_RESULT,
105107
),
106108
),
107109
"Immediately-completing child should emit its expected result",
@@ -357,15 +359,23 @@ suite("Roo Code Subtasks", function () {
357359

358360
await api.cancelCurrentTask()
359361

362+
// Gate on the async settle before asserting the parent never resumed: a spurious
363+
// resume would be an async downstream effect of the cancellation, so a synchronous
364+
// check right after cancelCurrentTask() proves nothing.
365+
await waitFor(() => api.getCurrentTaskStack().at(-1) === spawnedTaskId)
366+
await waitFor(
367+
() => asks[spawnedTaskId!]?.some(({ type, ask }) => type === "ask" && ask === "resume_task") ?? false,
368+
)
369+
360370
assert.ok(
361371
messages[parentTaskId]?.find(({ type, text }) => type === "say" && text === "Parent task resumed") ===
362372
undefined,
363373
"Parent task should not have resumed after subtask cancellation",
364374
)
365-
366-
await waitFor(() => api.getCurrentTaskStack().at(-1) === spawnedTaskId)
367-
await waitFor(
368-
() => asks[spawnedTaskId!]?.some(({ type, ask }) => type === "ask" && ask === "resume_task") ?? false,
375+
assert.strictEqual(
376+
messages[parentTaskId]?.find(({ say }) => say === "completion_result"),
377+
undefined,
378+
"Parent must not have completed after subtask cancellation",
369379
)
370380

371381
await api.clearCurrentTask()
@@ -539,15 +549,15 @@ suite("Roo Code Subtasks", function () {
539549
says[childTaskId!]
540550
?.filter(({ say }) => say === "completion_result")
541551
.map(({ text }) => text?.trim())
542-
.find((text) => text === SUBTASK_API_HANG_CHILD_RESULT),
552+
.find((text): text is string => !!text),
543553
SUBTASK_API_HANG_CHILD_RESULT,
544554
"Child should complete with its expected result after resume",
545555
)
546556
assert.strictEqual(
547557
says[parentTaskId]
548558
?.filter(({ say }) => say === "completion_result")
549559
.map(({ text }) => text?.trim())
550-
.find((text) => text === SUBTASK_API_HANG_PARENT_RESULT),
560+
.find((text): text is string => !!text),
551561
SUBTASK_API_HANG_PARENT_RESULT,
552562
"Parent should resume and complete with its expected result",
553563
)

src/__tests__/history-resume-delegation.spec.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,12 @@ describe("History resume delegation - parent metadata transitions", () => {
345345
expect(injectedMsg.role).toBe("user")
346346
expect((injectedMsg.content[0] as any).type).toBe("tool_result")
347347
expect((injectedMsg.content[0] as any).tool_use_id).toBe("toolu_abc123")
348+
349+
// Format contract with the e2e mock fixtures: the parent-resume fixtures in
350+
// apps/vscode-e2e/src/fixtures/subtasks.ts match on this injected
351+
// "completed.\n\nResult:" prefix (SUBTASK_RESULT_INJECTION). If this template
352+
// changes, update the fixtures in the same PR or they silently never fire.
353+
expect((injectedMsg.content[0] as any).content).toMatch(/^Subtask .+ completed\.\n\nResult:\n/)
348354
})
349355

350356
it("reopenParentFromDelegation injects plain text when no new_task tool_use exists in API history", async () => {

0 commit comments

Comments
 (0)