[Fix] Flaky mocked e2e subtasks test when parent resume is served the child fixture by zoomote[bot] · Pull Request #1002 · Zoo-Code-Org/Zoo-Code

zoomote · 2026-07-24T14:45:36Z

Opened on behalf of Elliott de Launay. Follow up by mentioning @zoomote, in the web UI, or in Discord.

Related GitHub Issue

Closes: #1001

Description

The mocked e2e test Roo Code Subtasks > child completing on its first response returns to parent flaked in CI (run 30098606515, merge queue for the unrelated PR #996; a re-run of the same SHA passed): the parent completed as "Fast child completed" instead of "Fast parent resumed" because aimock served the child fixture for the parent's resume request.

The fixture weaknesses in apps/vscode-e2e/src/fixtures/subtasks.ts, all fixed here:

Child fixtures matched parent-resume turns. The fast-child and SUBTASK_CHILD fixtures matched a bare marker that the corresponding parent prompt embeds verbatim. When the parent's tool result is serialized as a role: "tool" message, the original parent prompt (marker included) becomes the last user message and the child fixture — registered earlier — wins. Both fixtures are now predicates that keep aimock's last-user-message scoping (text content parts only, mirroring aimock's getTextContent) but exclude requests containing the parent marker, mirroring the [Flaky Test] e2e-mock: subtasks cross-profile fixture collision + task identity prompt timeout #561 fix for the cross-profile fixtures.
Parent-resume fixtures could miss or misfire. They guarded on the literal new_task tool-call id (rewritable by validateAndFixToolResultIds) or on the child result text (embedded verbatim in the parent prompts, so it can also match the parent's initial request on a turn-1 retry). All parent-resume fixtures now match on the injected subtask-result prefix (Subtask <id> completed.\n\nResult:\n), which can only appear after a child actually returns. The regular parent turn-1 fixture — shared by four tests, so sequenceIndex is not viable — excludes resume turns via the same injected prefix instead of relying on registration order.
The fixture/production format contract is now pinned. A new assertion in src/__tests__/history-resume-delegation.spec.ts verifies reopenParentFromDelegation's injected tool_result content matches the completed.\n\nResult: format the fixtures depend on, so a production template change fails loudly instead of silently starving the fixtures.

Also includes test-quality hardening from review: an exported SUBTASK_FAST_CHILD_RESULT constant, non-tautological result assertions in the api-hang test, and the no-parent-resume assertion in the cancellation test moved behind the async settle gates.

Test Procedure

TEST_FILE=subtasks.test xvfb-run -a pnpm --filter @roo-code/vscode-e2e test:ci:mock — 9/9 subtask e2e tests pass, including the previously failing one.
pnpm test (full unit suite) — 6851 passed, including the new injection-format contract assertion.
pnpm --filter @roo-code/vscode-e2e check-types and lint — clean.

Pre-Submission Checklist

Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
Scope: My changes are focused on the linked issue (one major feature/fix per PR).
Self-Review: I have performed a thorough self-review of my code.
Testing: New and/or updated tests have been added to cover my changes (if applicable).
Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

N/A — test-infrastructure-only change, no UI surface.

Documentation Updates

No documentation updates are required.

Additional Notes

Test-fixture-only change (plus one unit-test assertion); no product behavior is affected.

Get in Touch

edelauna

…nt-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

zoomote · 2026-07-24T14:45:48Z

No new code issues found. Both prior findings are resolved in the latest update. See task

apps/vscode-e2e/src/fixtures/subtasks.ts:172-190 — The regular parent turn-1 fixture (userMessage: /SUBTASK_PARENT_MARKER/) is the only parent turn-1 fixture without sequenceIndex: 0. In the role: "tool" resume serialization that caused this flake class, the last user message is the original parent prompt, so this fixture matches the parent's resume turn and re-serves new_task before the guarded child fixture (line 178) and parent-resume fixture (line 199) are ever evaluated — aimock serves the first content match in registration order. The new guards at 178/199 are therefore unreachable in exactly the path that motivated them; add sequenceIndex: 0 here for parity with the fast/interrupt/api-hang/abandon/xprofile parent fixtures. — Fixed in 68bae51: the fixture now excludes resume turns via the injected completed.\n\nResult: prefix (SUBTASK_RESULT_INJECTION), an equivalent guard since sequenceIndex is not viable for a fixture shared by four tests.
apps/vscode-e2e/src/fixtures/subtasks.ts:155,289 — The fast parent-resume guard [SUBTASK_FAST_PARENT_MARKER, "Fast child completed"] can also match the parent's initial request, because SUBTASK_FAST_PARENT_PROMPT embeds the child prompt verbatim and the child prompt itself contains Fast child completed (same for the api-hang guard [SUBTASK_API_HANG_PARENT_MARKER, SUBTASK_API_HANG_CHILD_RESULT] at line 271, where this PR removed the tool-call-id condition). Correctness now depends on the earlier sequenceIndex: 0 fixture winning turn 1; on a retry of the parent's first call (turn-1 fixture consumed, no tool result yet) these fixtures would serve the parent-completion response prematurely. The SUBTASK_RESULT_INJECTION guard used for the regular/interrupt resumes cannot match turn 1 — consider using it here too. — Fixed in 68bae51: both guards now match on SUBTASK_RESULT_INJECTION, which can only appear after a child actually returns.

_{Reviewed 68bae51}

codecov · 2026-07-24T14:47:30Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

zoomote · 2026-07-25T03:00:47Z

All review findings are addressed in 68bae51, pushed to this branch:

Review summary items

Regular parent turn-1 fixture: now excludes resume turns via the injected completed.\n\nResult: prefix instead of relying on registration order. Note: sequenceIndex: 0 is not viable for this fixture — it's shared by four tests — so the injection-prefix guard achieves the same goal without starving later tests.
Fast and api-hang parent-resume guards: now match SUBTASK_RESULT_INJECTION instead of the child result text (which is embedded verbatim in the parent prompts and could match a turn-1 retry).

Second review round

High: added a unit-test assertion pinning reopenParentFromDelegation's injected tool_result format to the completed.\n\nResult: contract the fixtures match on (src/__tests__/history-resume-delegation.spec.ts).
lastUserMessageContains now mirrors aimock's getTextContent (text content parts only).
Extracted SUBTASK_FAST_CHILD_RESULT; replaced the tautological .find()/assert pairs in the api-hang test; moved the no-parent-resume assertion in the cancellation test behind the async settle gates and added a completion_result absence check.

Validation: 9/9 subtask e2e tests in mock mode, full unit suite green (6851 tests), typecheck + lint clean.

…es, 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

zoomote · 2026-07-25T03:22:33Z

Re: the review summary above — both checklist items are addressed in 68bae51. One deviation worth noting: sequenceIndex: 0 is not viable for the regular parent turn-1 fixture because it is shared by four tests in the suite (only the first test's parent would ever match). Instead, that fixture now excludes resume turns via the same SUBTASK_RESULT_INJECTION guard used everywhere else — resume turns carry the injected completed.\n\nResult: tool_result, initial turns never do. The fast and api-hang parent-resume guards now match that injection prefix as well, so turn-1 retries can no longer be served a completion response.

zoomote · 2026-07-25T03:22:45Z