[Fix] Flaky mocked e2e subtasks test when parent resume is served the child fixture

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

@@ -18,7 +18,8 @@ const SUBTASK_XPROFILE_DIFFERENT_CHILD_MARKER = "SUBTASK_CHILD_DIFFERENT_PROFILE
 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.`
 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.`
 export const SUBTASK_CHILD_FOLLOWUP_ANSWER = "9"
-const SUBTASK_FAST_CHILD_PROMPT = `${SUBTASK_FAST_CHILD_MARKER}: Complete immediately with the exact result "Fast child completed".`
+export const SUBTASK_FAST_CHILD_RESULT = "Fast child completed"
+const SUBTASK_FAST_CHILD_PROMPT = `${SUBTASK_FAST_CHILD_MARKER}: Complete immediately with the exact result "${SUBTASK_FAST_CHILD_RESULT}".`
 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.`
 
 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[]) => {
 	return expected.every((text) => rawRequest.includes(text))
 }
 
+// aimock's `userMessage` matcher only inspects the LAST user message and joins only the
+// `type: "text"` content parts (see getTextContent in aimock's router). Fixtures that need
+// whole-request exclusions must replicate that scoping inside a predicate so they keep the
+// same matching semantics as the bare-regex fixtures they replace.
+const lastUserMessageContains = (req: ChatCompletionRequest, text: string) => {
+	const userMessages = req.messages?.filter((message) => message.role === "user") ?? []
+	const last = userMessages.at(-1)
+	if (!last) return false
+	const content =
+		typeof last.content === "string"
+			? last.content
+			: (last.content ?? [])
+					.filter((part): part is { type: "text"; text: string } => part?.type === "text")
+					.map((part) => part.text)
+					.join("")
+	return content.includes(text)
+}
+
+// reopenParentFromDelegation injects the child result into the resumed parent's history as
+// `Subtask <childId> completed.\n\nResult:\n<summary>`. Matching on this injected prefix (in
+// its JSON-serialized form) keeps parent-resume fixtures robust when
+// validateAndFixToolResultIds rewrites tool-use ids on resume — matching on the new_task
+// tool-call id directly proved flaky (the id can be rewritten, the fixture then misses, and
+// a looser child fixture wins and serves the child's response to the parent).
+const SUBTASK_RESULT_INJECTION = "completed.\\n\\nResult:"
 const completionAfterAnswer = (followupId: string, completionId: string) => ({
 	match: {
 		predicate: (req: ChatCompletionRequest) =>
@@ -100,25 +126,33 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
 		},
 	})
 
+	// The parent prompt embeds SUBTASK_FAST_CHILD_MARKER verbatim, so parent-resume turns
+	// can also match a bare substring check (same collision class as #561). Exclude the
+	// parent marker so those turns fall through to the parent-resume fixture below.
 	mock.addFixture({
 		match: {
-			userMessage: new RegExp(SUBTASK_FAST_CHILD_MARKER),
+			predicate: (req: ChatCompletionRequest) =>
+				lastUserMessageContains(req, SUBTASK_FAST_CHILD_MARKER) &&
+				!requestContains(req, [SUBTASK_FAST_PARENT_MARKER]),
 		},
 		response: {
 			toolCalls: [
 				{
 					name: "attempt_completion",
-					arguments: JSON.stringify({ result: "Fast child completed" }),
+					arguments: JSON.stringify({ result: SUBTASK_FAST_CHILD_RESULT }),
 					id: "call_subtasks_fast_child_completion_002",
 				},
 			],
 		},
 	})
 
+	// Guard on SUBTASK_RESULT_INJECTION (not the child result text): the child result is
+	// embedded verbatim in SUBTASK_FAST_PARENT_PROMPT, so it cannot distinguish the parent's
+	// initial request from its resume turn.
 	mock.addFixture({
 		match: {
 			predicate: (req: ChatCompletionRequest) =>
-				requestContains(req, [SUBTASK_FAST_PARENT_MARKER, "call_subtasks_fast_parent_new_task_001"]),
+				requestContains(req, [SUBTASK_FAST_PARENT_MARKER, SUBTASK_RESULT_INJECTION]),
 		},
 		response: {
 			toolCalls: [
@@ -131,9 +165,15 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
 		},
 	})
 
+	// This fixture is shared by several tests, so sequenceIndex cannot guard it. Exclude
+	// resume turns via the injected tool_result prefix instead: when the tool_result is
+	// serialized as role:"tool", the original parent prompt is the last user message again
+	// and would otherwise re-serve new_task on the parent's resume turn.
 	mock.addFixture({
 		match: {
-			userMessage: new RegExp(SUBTASK_PARENT_MARKER),
+			predicate: (req: ChatCompletionRequest) =>
+				lastUserMessageContains(req, SUBTASK_PARENT_MARKER) &&
+				!requestContains(req, [SUBTASK_RESULT_INJECTION]),
 		},
 		response: {
 			toolCalls: [
@@ -149,9 +189,12 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
 		},
 	})
 
+	// Same collision guard as the fast-child fixture above: SUBTASK_PARENT_PROMPT embeds
+	// SUBTASK_CHILD_MARKER verbatim, so parent-resume turns must not match this fixture.
 	mock.addFixture({
 		match: {
-			userMessage: new RegExp(SUBTASK_CHILD_MARKER),
+			predicate: (req: ChatCompletionRequest) =>
+				lastUserMessageContains(req, SUBTASK_CHILD_MARKER) && !requestContains(req, [SUBTASK_PARENT_MARKER]),
 		},
 		response: {
 			toolCalls: [
@@ -172,7 +215,7 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
 	mock.addFixture({
 		match: {
 			predicate: (req: ChatCompletionRequest) =>
-				requestContains(req, [SUBTASK_PARENT_MARKER, "call_subtasks_parent_new_task_001"]),
+				requestContains(req, [SUBTASK_PARENT_MARKER, SUBTASK_RESULT_INJECTION]),
 		},
 		response: {
 			toolCalls: [
@@ -238,14 +281,12 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
 		},
 	})
 
+	// Same as the fast parent-resume fixture: the child result is embedded verbatim in
+	// SUBTASK_API_HANG_PARENT_PROMPT, so guard on the injected tool_result prefix instead.
 	mock.addFixture({
 		match: {
 			predicate: (req: ChatCompletionRequest) =>
-				requestContains(req, [
-					SUBTASK_API_HANG_PARENT_MARKER,
-					"call_api_hang_parent_new_task_001",
-					SUBTASK_API_HANG_CHILD_RESULT,
-				]),
+				requestContains(req, [SUBTASK_API_HANG_PARENT_MARKER, SUBTASK_RESULT_INJECTION]),
 		},
 		response: {
 			toolCalls: [
@@ -433,7 +474,7 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
 	mock.addFixture({
 		match: {
 			predicate: (req: ChatCompletionRequest) =>
-				requestContains(req, [SUBTASK_INTERRUPT_PARENT_MARKER, "call_interrupt_parent_new_task_001"]),
+				requestContains(req, [SUBTASK_INTERRUPT_PARENT_MARKER, SUBTASK_RESULT_INJECTION]),
 		},
 		response: {
 			toolCalls: [

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

@@ -14,6 +14,7 @@ import {
 	SUBTASK_API_HANG_PARENT_RESULT,
 	SUBTASK_API_HANG_RESUME_MESSAGE,
 	SUBTASK_CHILD_FOLLOWUP_ANSWER,
+	SUBTASK_FAST_CHILD_RESULT,
 	SUBTASK_FAST_PARENT_PROMPT,
 	SUBTASK_INTERRUPT_CHILD_FOLLOWUP_ANSWER,
 	SUBTASK_INTERRUPT_PARENT_PROMPT,
@@ -101,7 +102,8 @@ suite("Roo Code Subtasks", function () {
 					([taskId, messages]) =>
 						taskId !== parentTaskId &&
 						messages.some(
-							({ say, text }) => say === "completion_result" && text?.trim() === "Fast child completed",
+							({ say, text }) =>
+								say === "completion_result" && text?.trim() === SUBTASK_FAST_CHILD_RESULT,
 						),
 				),
 				"Immediately-completing child should emit its expected result",
@@ -357,15 +359,23 @@ suite("Roo Code Subtasks", function () {
 
 			await api.cancelCurrentTask()
 
+			// Gate on the async settle before asserting the parent never resumed: a spurious
+			// resume would be an async downstream effect of the cancellation, so a synchronous
+			// check right after cancelCurrentTask() proves nothing.
+			await waitFor(() => api.getCurrentTaskStack().at(-1) === spawnedTaskId)
+			await waitFor(
+				() => asks[spawnedTaskId!]?.some(({ type, ask }) => type === "ask" && ask === "resume_task") ?? false,
+			)
+
 			assert.ok(
 				messages[parentTaskId]?.find(({ type, text }) => type === "say" && text === "Parent task resumed") ===
 					undefined,
 				"Parent task should not have resumed after subtask cancellation",
 			)
-
-			await waitFor(() => api.getCurrentTaskStack().at(-1) === spawnedTaskId)
-			await waitFor(
-				() => asks[spawnedTaskId!]?.some(({ type, ask }) => type === "ask" && ask === "resume_task") ?? false,
+			assert.strictEqual(
+				messages[parentTaskId]?.find(({ say }) => say === "completion_result"),
+				undefined,
+				"Parent must not have completed after subtask cancellation",
 			)
 
 			await api.clearCurrentTask()
@@ -539,15 +549,15 @@ suite("Roo Code Subtasks", function () {
 				says[childTaskId!]
 					?.filter(({ say }) => say === "completion_result")
 					.map(({ text }) => text?.trim())
-					.find((text) => text === SUBTASK_API_HANG_CHILD_RESULT),
+					.find((text): text is string => !!text),
 				SUBTASK_API_HANG_CHILD_RESULT,
 				"Child should complete with its expected result after resume",
 			)
 			assert.strictEqual(
 				says[parentTaskId]
 					?.filter(({ say }) => say === "completion_result")
 					.map(({ text }) => text?.trim())
-					.find((text) => text === SUBTASK_API_HANG_PARENT_RESULT),
+					.find((text): text is string => !!text),
 				SUBTASK_API_HANG_PARENT_RESULT,
 				"Parent should resume and complete with its expected result",
 			)

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

@@ -345,6 +345,12 @@ describe("History resume delegation - parent metadata transitions", () => {
 		expect(injectedMsg.role).toBe("user")
 		expect((injectedMsg.content[0] as any).type).toBe("tool_result")
 		expect((injectedMsg.content[0] as any).tool_use_id).toBe("toolu_abc123")
+
+		// Format contract with the e2e mock fixtures: the parent-resume fixtures in
+		// apps/vscode-e2e/src/fixtures/subtasks.ts match on this injected
+		// "completed.\n\nResult:" prefix (SUBTASK_RESULT_INJECTION). If this template
+		// changes, update the fixtures in the same PR or they silently never fire.
+		expect((injectedMsg.content[0] as any).content).toMatch(/^Subtask .+ completed\.\n\nResult:\n/)
 	})
 
 	it("reopenParentFromDelegation injects plain text when no new_task tool_use exists in API history", async () => {