Skip to content

Commit 68bae51

Browse files
committed
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
1 parent dcae4b7 commit 68bae51

3 files changed

Lines changed: 49 additions & 15 deletions

File tree

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

Lines changed: 26 additions & 8 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,14 +55,21 @@ const requestContains = (req: ChatCompletionRequest, expected: string[]) => {
5455
return expected.every((text) => rawRequest.includes(text))
5556
}
5657

57-
// aimock's `userMessage` matcher only inspects the LAST user message. Fixtures that need
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
5860
// whole-request exclusions must replicate that scoping inside a predicate so they keep the
5961
// same matching semantics as the bare-regex fixtures they replace.
6062
const lastUserMessageContains = (req: ChatCompletionRequest, text: string) => {
6163
const userMessages = req.messages?.filter((message) => message.role === "user") ?? []
6264
const last = userMessages.at(-1)
6365
if (!last) return false
64-
const content = typeof last.content === "string" ? last.content : JSON.stringify(last.content ?? "")
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("")
6573
return content.includes(text)
6674
}
6775

@@ -72,7 +80,6 @@ const lastUserMessageContains = (req: ChatCompletionRequest, text: string) => {
7280
// tool-call id directly proved flaky (the id can be rewritten, the fixture then misses, and
7381
// a looser child fixture wins and serves the child's response to the parent).
7482
const SUBTASK_RESULT_INJECTION = "completed.\\n\\nResult:"
75-
7683
const completionAfterAnswer = (followupId: string, completionId: string) => ({
7784
match: {
7885
predicate: (req: ChatCompletionRequest) =>
@@ -132,17 +139,20 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
132139
toolCalls: [
133140
{
134141
name: "attempt_completion",
135-
arguments: JSON.stringify({ result: "Fast child completed" }),
142+
arguments: JSON.stringify({ result: SUBTASK_FAST_CHILD_RESULT }),
136143
id: "call_subtasks_fast_child_completion_002",
137144
},
138145
],
139146
},
140147
})
141148

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.
142152
mock.addFixture({
143153
match: {
144154
predicate: (req: ChatCompletionRequest) =>
145-
requestContains(req, [SUBTASK_FAST_PARENT_MARKER, "Fast child completed"]),
155+
requestContains(req, [SUBTASK_FAST_PARENT_MARKER, SUBTASK_RESULT_INJECTION]),
146156
},
147157
response: {
148158
toolCalls: [
@@ -155,9 +165,15 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
155165
},
156166
})
157167

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.
158172
mock.addFixture({
159173
match: {
160-
userMessage: new RegExp(SUBTASK_PARENT_MARKER),
174+
predicate: (req: ChatCompletionRequest) =>
175+
lastUserMessageContains(req, SUBTASK_PARENT_MARKER) &&
176+
!requestContains(req, [SUBTASK_RESULT_INJECTION]),
161177
},
162178
response: {
163179
toolCalls: [
@@ -265,10 +281,12 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
265281
},
266282
})
267283

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.
268286
mock.addFixture({
269287
match: {
270288
predicate: (req: ChatCompletionRequest) =>
271-
requestContains(req, [SUBTASK_API_HANG_PARENT_MARKER, SUBTASK_API_HANG_CHILD_RESULT]),
289+
requestContains(req, [SUBTASK_API_HANG_PARENT_MARKER, SUBTASK_RESULT_INJECTION]),
272290
},
273291
response: {
274292
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)