Skip to content

[Fix] Subtask e2e suite can inherit a cancelled delayed mock stream from the previous test #1074

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
zoomote wants to merge 1 commit into
base: main
Choose a base branch
from
+75 −16
Draft

[Fix] Subtask e2e suite can inherit a cancelled delayed mock stream from the previous test #1074

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions apps/vscode-e2e/src/fixtures/subtasks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ export const SUBTASK_API_HANG_RESUME_MESSAGE = "Continue after provider hang."
export const SUBTASK_API_HANG_CHILD_RESULT = "Hung child completed"
export const SUBTASK_API_HANG_PARENT_RESULT = "API hang parent resumed"

// How long the API-hang child's first mocked response stays pending before its first SSE
// chunk. Shared with the subtask suite so its post-test drain waits exactly one window.
export const SUBTASK_API_HANG_RESPONSE_LATENCY_MS = 15_000

// Abandon-subtask scenario (#559) — separate markers to avoid sequenceIndex collisions with the
// interrupted-child-resumes tests above, which exhaust the sequence count for INTERRUPT markers.
const SUBTASK_ABANDON_PARENT_MARKER = "SUBTASK_PARENT_ABANDON_SEVER"
Expand Down Expand Up @@ -261,8 +265,15 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
userMessage: apiHangChildMatch,
sequenceIndex: 0,
},
// Keep the first child response pending long enough for the e2e test to cancel an in-flight API request.
latency: 15_000,
// Keep the first child response pending long enough for the e2e test to cancel an in-flight
// API request. Delay only the first chunk (ttft) rather than using flat `latency`: aimock
// applies `latency` to EVERY chunk and never observes client disconnects, so after the test
// cancels, a flat-latency stream would stay pending server-side for chunks × latency before
// flushing to the dead socket. With ttft the pending window is exactly
// SUBTASK_API_HANG_RESPONSE_LATENCY_MS, which is what the suite's post-test drain waits out.
// This relies on no flat `latency` (fixture or LLMock default) being set — a flat latency
// would still apply to every chunk after the first.
streamingProfile: { ttft: SUBTASK_API_HANG_RESPONSE_LATENCY_MS },
response: {
toolCalls: [
{
Expand Down
76 changes: 62 additions & 14 deletions apps/vscode-e2e/src/suite/subtasks.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
SUBTASK_API_HANG_PARENT_MARKER,
SUBTASK_API_HANG_PARENT_PROMPT,
SUBTASK_API_HANG_PARENT_RESULT,
SUBTASK_API_HANG_RESPONSE_LATENCY_MS,
SUBTASK_API_HANG_RESUME_MESSAGE,
SUBTASK_CHILD_FOLLOWUP_ANSWER,
SUBTASK_FAST_CHILD_RESULT,
Expand All @@ -33,6 +34,7 @@ import {
type AimockMessageContent = string | Array<{ type?: string; text?: string }>

type AimockJournalEntry = {
timestamp?: number
body?: {
messages?: Array<{
role?: string
Expand All @@ -49,24 +51,63 @@ const messageContentText = (content?: AimockMessageContent) => {
return content?.map((part) => part.text ?? "").join("") ?? ""
}

const waitForAimockRequestContaining = async (expectedText: string, excludeText?: string) => {
const fetchAimockJournal = async () => {
const aimockUrl = process.env.AIMOCK_URL
assert.ok(aimockUrl, "AIMOCK_URL must be set for aimock journal assertions")

const response = await fetch(`${aimockUrl}/__aimock/journal`)
return (await response.json()) as AimockJournalEntry[]
}

const findAimockRequest = (entries: AimockJournalEntry[], expectedText: string, excludeText?: string) =>
entries.find((entry) => {
const messages = entry.body?.messages
if (!messages) return false
const entryText = messages.map((m) => messageContentText(m.content)).join("")
if (excludeText && entryText.includes(excludeText)) return false
return messages.some(
(message) => message.role === "user" && messageContentText(message.content).includes(expectedText),
)
})

// Returns the journal timestamp of the matching request so callers can anchor
// post-test drains to the exact request this test created.
const waitForAimockRequestContaining = async (expectedText: string, excludeText?: string) => {
let matchedAt: number | undefined

await waitFor(async () => {
const response = await fetch(`${aimockUrl}/__aimock/journal`)
const entries = (await response.json()) as AimockJournalEntry[]

return entries.some((entry) => {
const messages = entry.body?.messages
if (!messages) return false
const entryText = messages.map((m) => messageContentText(m.content)).join("")
if (excludeText && entryText.includes(excludeText)) return false
return messages.some(
(message) => message.role === "user" && messageContentText(message.content).includes(expectedText),
)
})
matchedAt = findAimockRequest(await fetchAimockJournal(), expectedText, excludeText)?.timestamp
return matchedAt !== undefined
})

return matchedAt
}

// Grace period after the delayed window for aimock to flush the stream's remaining
// chunks to the dead socket.
const SUBTASK_API_HANG_DRAIN_GRACE_MS = 500

// aimock does not observe client disconnects: after the API-hang child request is cancelled,
// the mock keeps the delayed stream pending server-side until the fixture's ttft has fully
// elapsed, then flushes the remaining chunks to the dead socket. A streamed request opened by
// the next test can interleave with that late flush, so wait out the remainder of the delayed
// window before the next test runs. The deadline is anchored to the journal timestamp of the
// request this test created (never earlier traffic), and bounded by one latency window plus
// grace, so it cannot hide a genuine hang.
const waitForDelayedSubtaskStreamDrain = async (delayedRequestStartedAt: number | undefined) => {
if (delayedRequestStartedAt === undefined) {
// The delayed request never reached the mock (the test failed before cancelling
// an in-flight request), so there is no delayed stream to drain.
return
}

const drainDeadlineMs =
delayedRequestStartedAt + SUBTASK_API_HANG_RESPONSE_LATENCY_MS + SUBTASK_API_HANG_DRAIN_GRACE_MS
const remainingMs = drainDeadlineMs - Date.now()

if (remainingMs > 0) {
await sleep(remainingMs)
}
}

suite("Roo Code Subtasks", function () {
Expand Down Expand Up @@ -482,6 +523,7 @@ suite("Roo Code Subtasks", function () {
const api = globalThis.api
const asks: Record<string, ClineMessage[]> = {}
const says: Record<string, ClineMessage[]> = {}
let delayedChildRequestStartedAt: number | undefined

const messageHandler = ({ taskId, message }: { taskId: string; message: ClineMessage }) => {
if (message.type === "ask") {
Expand Down Expand Up @@ -519,7 +561,10 @@ suite("Roo Code Subtasks", function () {
return false
})

await waitForAimockRequestContaining(SUBTASK_API_HANG_CHILD_MARKER, SUBTASK_API_HANG_PARENT_MARKER)
delayedChildRequestStartedAt = await waitForAimockRequestContaining(
SUBTASK_API_HANG_CHILD_MARKER,
SUBTASK_API_HANG_PARENT_MARKER,
)

await api.cancelCurrentTask()

Expand Down Expand Up @@ -580,6 +625,9 @@ suite("Roo Code Subtasks", function () {
await api.clearCurrentTask()
}
await waitFor(() => api.getCurrentTaskStack().length === 0).catch(() => {})
// Drain the cancelled delayed stream before the next test can open another
// streamed request against the mock.
await waitForDelayedSubtaskStreamDrain(delayedChildRequestStartedAt)
}
})

Expand Down
Loading