From dcae4b7b3a1b56eddaeb486f8ea4ce8a962cf7a5 Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 24 Jul 2026 14:42:39 +0000 Subject: [PATCH 1/2] 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 --- apps/vscode-e2e/src/fixtures/subtasks.ts | 43 ++++++++++++++++++------ 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/apps/vscode-e2e/src/fixtures/subtasks.ts b/apps/vscode-e2e/src/fixtures/subtasks.ts index 5a39577dd9..b9118f65a8 100644 --- a/apps/vscode-e2e/src/fixtures/subtasks.ts +++ b/apps/vscode-e2e/src/fixtures/subtasks.ts @@ -54,6 +54,25 @@ const requestContains = (req: ChatCompletionRequest, expected: string[]) => { return expected.every((text) => rawRequest.includes(text)) } +// aimock's `userMessage` matcher only inspects the LAST user message. 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 : JSON.stringify(last.content ?? "") + return content.includes(text) +} + +// reopenParentFromDelegation injects the child result into the resumed parent's history as +// `Subtask completed.\n\nResult:\n`. 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,9 +119,14 @@ export function addSubtaskFixtures(mock: InstanceType) { }, }) + // 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: [ @@ -118,7 +142,7 @@ export function addSubtaskFixtures(mock: InstanceType) { 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, "Fast child completed"]), }, response: { toolCalls: [ @@ -149,9 +173,12 @@ export function addSubtaskFixtures(mock: InstanceType) { }, }) + // 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 +199,7 @@ export function addSubtaskFixtures(mock: InstanceType) { 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: [ @@ -241,11 +268,7 @@ export function addSubtaskFixtures(mock: InstanceType) { 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_API_HANG_CHILD_RESULT]), }, response: { toolCalls: [ @@ -433,7 +456,7 @@ export function addSubtaskFixtures(mock: InstanceType) { 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: [ From 68bae5163f4ed4e550ffa29caa0d8b162e0b5cb4 Mon Sep 17 00:00:00 2001 From: Roomote Date: Sat, 25 Jul 2026 03:19:12 +0000 Subject: [PATCH 2/2] =?UTF-8?q?test(e2e-mock):=20address=20PR=20review=20?= =?UTF-8?q?=E2=80=94=20injection-guard=20all=20parent=20fixtures,=20link?= =?UTF-8?q?=20fixture=20format=20contract?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- apps/vscode-e2e/src/fixtures/subtasks.ts | 34 ++++++++++++++----- apps/vscode-e2e/src/suite/subtasks.test.ts | 24 +++++++++---- .../history-resume-delegation.spec.ts | 6 ++++ 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/apps/vscode-e2e/src/fixtures/subtasks.ts b/apps/vscode-e2e/src/fixtures/subtasks.ts index b9118f65a8..890c243b38 100644 --- a/apps/vscode-e2e/src/fixtures/subtasks.ts +++ b/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,14 +55,21 @@ const requestContains = (req: ChatCompletionRequest, expected: string[]) => { return expected.every((text) => rawRequest.includes(text)) } -// aimock's `userMessage` matcher only inspects the LAST user message. Fixtures that need +// 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 : JSON.stringify(last.content ?? "") + 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) } @@ -72,7 +80,6 @@ const lastUserMessageContains = (req: ChatCompletionRequest, text: string) => { // 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) => @@ -132,17 +139,20 @@ export function addSubtaskFixtures(mock: InstanceType) { 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, "Fast child completed"]), + requestContains(req, [SUBTASK_FAST_PARENT_MARKER, SUBTASK_RESULT_INJECTION]), }, response: { toolCalls: [ @@ -155,9 +165,15 @@ export function addSubtaskFixtures(mock: InstanceType) { }, }) + // 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: [ @@ -265,10 +281,12 @@ export function addSubtaskFixtures(mock: InstanceType) { }, }) + // 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, SUBTASK_API_HANG_CHILD_RESULT]), + requestContains(req, [SUBTASK_API_HANG_PARENT_MARKER, SUBTASK_RESULT_INJECTION]), }, response: { toolCalls: [ diff --git a/apps/vscode-e2e/src/suite/subtasks.test.ts b/apps/vscode-e2e/src/suite/subtasks.test.ts index 661d7f0e2d..641e02f6ac 100644 --- a/apps/vscode-e2e/src/suite/subtasks.test.ts +++ b/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,7 +549,7 @@ 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", ) @@ -547,7 +557,7 @@ suite("Roo Code Subtasks", function () { 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", ) diff --git a/src/__tests__/history-resume-delegation.spec.ts b/src/__tests__/history-resume-delegation.spec.ts index 9c54ba3301..fc496d0c84 100644 --- a/src/__tests__/history-resume-delegation.spec.ts +++ b/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 () => {