Skip to content

Commit f8302c0

Browse files
committed
Merge branch 'main' into fix/router-provider-context-window
2 parents 1264874 + eace967 commit f8302c0

32 files changed

Lines changed: 1086 additions & 123 deletions

apps/vscode-e2e/AGENTS.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,17 @@ Example:
121121
122122
The `model` field can be added to either match when a test targets a specific model.
123123
124+
## Delaying a fixture response (simulating a slow or hung provider)
125+
126+
Use `streamingProfile: { ttft: <ms> }` on a fixture, not a flat `latency: <ms>`, when a test needs
127+
to simulate a slow or hung provider (e.g. to cancel an in-flight request mid-stream). `ttft` delays
128+
only the first SSE chunk, so the pending window is exactly the configured value. Flat `latency`
129+
delays _every_ chunk, and aimock never observes client disconnects — after a test cancels the
130+
request, a flat-latency stream keeps flushing chunks server-side for `chunks × latency` before
131+
reaching the dead socket, which can interleave with the next test's request against the same mock
132+
server. See `SUBTASK_API_HANG_RESPONSE_LATENCY_MS` in `fixtures/subtasks.ts` for an example,
133+
including the bounded post-test drain the calling suite uses to wait out that window.
134+
124135
## 404 errors in logs are expected
125136

126137
Background API calls from the extension (usage collection, initialization) hit aimock with no matching fixture and return 404. These do **not** affect test results — the tests still pass. You'll see `[OpenRouter] API error: { message: '404 No fixture matched' }` in the output; this is normal.

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ export const SUBTASK_API_HANG_RESUME_MESSAGE = "Continue after provider hang."
3333
export const SUBTASK_API_HANG_CHILD_RESULT = "Hung child completed"
3434
export const SUBTASK_API_HANG_PARENT_RESULT = "API hang parent resumed"
3535

36+
// How long the API-hang child's first mocked response stays pending before its first SSE
37+
// chunk. Shared with the subtask suite so its post-test drain waits exactly one window.
38+
// Correctness depends on no flat `latency` (fixture or LLMock default) being set on that
39+
// fixture — a flat latency would apply to every chunk after the first, not just the ttft.
40+
export const SUBTASK_API_HANG_RESPONSE_LATENCY_MS = 15_000
41+
3642
// Abandon-subtask scenario (#559) — separate markers to avoid sequenceIndex collisions with the
3743
// interrupted-child-resumes tests above, which exhaust the sequence count for INTERRUPT markers.
3844
const SUBTASK_ABANDON_PARENT_MARKER = "SUBTASK_PARENT_ABANDON_SEVER"
@@ -261,8 +267,14 @@ export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
261267
userMessage: apiHangChildMatch,
262268
sequenceIndex: 0,
263269
},
264-
// Keep the first child response pending long enough for the e2e test to cancel an in-flight API request.
265-
latency: 15_000,
270+
// Keep the first child response pending long enough for the e2e test to cancel an in-flight
271+
// API request. Delay only the first chunk (ttft) rather than using flat `latency`: aimock
272+
// applies `latency` to EVERY chunk and never observes client disconnects, so after the test
273+
// cancels, a flat-latency stream would stay pending server-side for chunks × latency before
274+
// flushing to the dead socket. With ttft the pending window is exactly
275+
// SUBTASK_API_HANG_RESPONSE_LATENCY_MS (see its doc comment for the no-flat-latency
276+
// invariant this relies on), which is what the suite's post-test drain waits out.
277+
streamingProfile: { ttft: SUBTASK_API_HANG_RESPONSE_LATENCY_MS },
266278
response: {
267279
toolCalls: [
268280
{

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

Lines changed: 66 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
SUBTASK_API_HANG_PARENT_MARKER,
1717
SUBTASK_API_HANG_PARENT_PROMPT,
1818
SUBTASK_API_HANG_PARENT_RESULT,
19+
SUBTASK_API_HANG_RESPONSE_LATENCY_MS,
1920
SUBTASK_API_HANG_RESUME_MESSAGE,
2021
SUBTASK_CHILD_FOLLOWUP_ANSWER,
2122
SUBTASK_FAST_CHILD_RESULT,
@@ -33,6 +34,7 @@ import {
3334
type AimockMessageContent = string | Array<{ type?: string; text?: string }>
3435

3536
type AimockJournalEntry = {
37+
timestamp?: number
3638
body?: {
3739
messages?: Array<{
3840
role?: string
@@ -49,24 +51,67 @@ const messageContentText = (content?: AimockMessageContent) => {
4951
return content?.map((part) => part.text ?? "").join("") ?? ""
5052
}
5153

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

58+
const response = await fetch(`${aimockUrl}/__aimock/journal`)
59+
return (await response.json()) as AimockJournalEntry[]
60+
}
61+
62+
const findAimockRequest = (entries: AimockJournalEntry[], expectedText: string, excludeText?: string) =>
63+
entries.find((entry) => {
64+
const messages = entry.body?.messages
65+
if (!messages) return false
66+
const entryText = messages.map((m) => messageContentText(m.content)).join("")
67+
if (excludeText && entryText.includes(excludeText)) return false
68+
return messages.some(
69+
(message) => message.role === "user" && messageContentText(message.content).includes(expectedText),
70+
)
71+
})
72+
73+
// Waits for a matching request to appear in the aimock journal and returns its journal
74+
// timestamp, so callers can anchor post-test drains to the exact request this test created.
75+
const waitForAimockRequestContaining = async (
76+
expectedText: string,
77+
excludeText?: string,
78+
): Promise<number | undefined> => {
79+
let matchedAt: number | undefined
80+
5681
await waitFor(async () => {
57-
const response = await fetch(`${aimockUrl}/__aimock/journal`)
58-
const entries = (await response.json()) as AimockJournalEntry[]
59-
60-
return entries.some((entry) => {
61-
const messages = entry.body?.messages
62-
if (!messages) return false
63-
const entryText = messages.map((m) => messageContentText(m.content)).join("")
64-
if (excludeText && entryText.includes(excludeText)) return false
65-
return messages.some(
66-
(message) => message.role === "user" && messageContentText(message.content).includes(expectedText),
67-
)
68-
})
82+
matchedAt = findAimockRequest(await fetchAimockJournal(), expectedText, excludeText)?.timestamp
83+
return matchedAt !== undefined
6984
})
85+
86+
return matchedAt
87+
}
88+
89+
// Grace period after the delayed window for aimock to flush the stream's remaining chunks to
90+
// the dead socket. 500ms is an empirical margin for that flush plus socket teardown; if this
91+
// suite becomes flaky again on slow CI runners, widen this value first.
92+
const SUBTASK_API_HANG_DRAIN_GRACE_MS = 500
93+
94+
// aimock does not observe client disconnects: after the API-hang child request is cancelled,
95+
// the mock keeps the delayed stream pending server-side until the fixture's ttft has fully
96+
// elapsed, then flushes the remaining chunks to the dead socket. A streamed request opened by
97+
// the next test can interleave with that late flush, so wait out the remainder of the delayed
98+
// window before the next test runs. The deadline is anchored to the journal timestamp of the
99+
// request this test created (never earlier traffic), and bounded by one latency window plus
100+
// grace, so it cannot hide a genuine hang.
101+
const waitForDelayedSubtaskStreamDrain = async (delayedRequestStartedAt: number | undefined) => {
102+
if (delayedRequestStartedAt === undefined) {
103+
// The delayed request never reached the mock (the test failed before cancelling
104+
// an in-flight request), so there is no delayed stream to drain.
105+
return
106+
}
107+
108+
const drainDeadlineMs =
109+
delayedRequestStartedAt + SUBTASK_API_HANG_RESPONSE_LATENCY_MS + SUBTASK_API_HANG_DRAIN_GRACE_MS
110+
const remainingMs = drainDeadlineMs - Date.now()
111+
112+
if (remainingMs > 0) {
113+
await sleep(remainingMs)
114+
}
70115
}
71116

72117
suite("Roo Code Subtasks", function () {
@@ -482,6 +527,7 @@ suite("Roo Code Subtasks", function () {
482527
const api = globalThis.api
483528
const asks: Record<string, ClineMessage[]> = {}
484529
const says: Record<string, ClineMessage[]> = {}
530+
let delayedChildRequestStartedAt: number | undefined
485531

486532
const messageHandler = ({ taskId, message }: { taskId: string; message: ClineMessage }) => {
487533
if (message.type === "ask") {
@@ -519,7 +565,10 @@ suite("Roo Code Subtasks", function () {
519565
return false
520566
})
521567

522-
await waitForAimockRequestContaining(SUBTASK_API_HANG_CHILD_MARKER, SUBTASK_API_HANG_PARENT_MARKER)
568+
delayedChildRequestStartedAt = await waitForAimockRequestContaining(
569+
SUBTASK_API_HANG_CHILD_MARKER,
570+
SUBTASK_API_HANG_PARENT_MARKER,
571+
)
523572

524573
await api.cancelCurrentTask()
525574

@@ -580,6 +629,9 @@ suite("Roo Code Subtasks", function () {
580629
await api.clearCurrentTask()
581630
}
582631
await waitFor(() => api.getCurrentTaskStack().length === 0).catch(() => {})
632+
// Drain the cancelled delayed stream before the next test can open another
633+
// streamed request against the mock.
634+
await waitForDelayedSubtaskStreamDrain(delayedChildRequestStartedAt)
583635
}
584636
})
585637

packages/types/package.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,14 @@
1111
"types": "./dist/index.d.cts",
1212
"default": "./dist/index.cjs"
1313
}
14+
},
15+
"./model": {
16+
"types": "./src/model.ts",
17+
"import": "./src/model.ts"
18+
},
19+
"./provider-identifiers": {
20+
"types": "./src/provider-identifiers.ts",
21+
"import": "./src/provider-identifiers.ts"
1422
}
1523
},
1624
"scripts": {

packages/types/src/__tests__/provider-settings.test.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,49 @@
1-
import { ANTHROPIC_API_PROTOCOL, getApiProtocol, OPENAI_API_PROTOCOL, providerIdentifiers } from "../index.js"
1+
import { ANTHROPIC_API_PROTOCOL, OPENAI_API_PROTOCOL, providerIdentifiers } from "../index.js"
2+
import {
3+
getApiProtocol,
4+
OPEN_AI_CODEX_SERVICE_TIER_KEY,
5+
PROVIDER_SETTINGS_KEYS,
6+
providerSettingsSchema,
7+
providerSettingsSchemaDiscriminated,
8+
} from "../provider-settings.js"
9+
import { OpenAiCodexServiceTier, OpenAiServiceTier } from "../model.js"
10+
11+
describe("OpenAI Codex provider settings", () => {
12+
it("preserves the Fast preference in general and provider-specific schemas", () => {
13+
const settings = {
14+
apiProvider: providerIdentifiers.openaiCodex,
15+
apiModelId: "gpt-5.6-sol",
16+
[OPEN_AI_CODEX_SERVICE_TIER_KEY]: OpenAiCodexServiceTier.Priority,
17+
}
18+
19+
expect(providerSettingsSchema.parse(settings)).toEqual(settings)
20+
expect(providerSettingsSchemaDiscriminated.parse(settings)).toEqual(settings)
21+
expect(PROVIDER_SETTINGS_KEYS).toContain(OPEN_AI_CODEX_SERVICE_TIER_KEY)
22+
})
23+
24+
it.each([undefined, OpenAiCodexServiceTier.Default])(
25+
"accepts %s as the Standard preference",
26+
(openAiCodexServiceTier) => {
27+
const standardSettings = {
28+
apiProvider: providerIdentifiers.openaiCodex,
29+
apiModelId: "gpt-5.6-sol",
30+
...(openAiCodexServiceTier ? { [OPEN_AI_CODEX_SERVICE_TIER_KEY]: openAiCodexServiceTier } : {}),
31+
}
32+
33+
expect(providerSettingsSchemaDiscriminated.parse(standardSettings)).toEqual(standardSettings)
34+
},
35+
)
36+
37+
it("rejects unsupported service tiers", () => {
38+
expect(
39+
providerSettingsSchemaDiscriminated.safeParse({
40+
apiProvider: providerIdentifiers.openaiCodex,
41+
apiModelId: "gpt-5.6-sol",
42+
[OPEN_AI_CODEX_SERVICE_TIER_KEY]: OpenAiServiceTier.Flex,
43+
}).success,
44+
).toBe(false)
45+
})
46+
})
247

348
describe("getApiProtocol", () => {
449
it("preserves API protocol wire values", () => {

packages/types/src/model.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,34 @@ export const verbosityLevelsSchema = z.enum(verbosityLevels)
5454

5555
export type VerbosityLevel = z.infer<typeof verbosityLevelsSchema>
5656

57+
/** Serialized service tier field used in provider request payloads and responses. */
58+
export const SERVICE_TIER_KEY = "service_tier"
59+
5760
/**
58-
* Service tiers (OpenAI Responses API)
61+
* Service tiers for the public OpenAI Responses API.
5962
*/
60-
export const serviceTiers = ["default", "flex", "priority"] as const
63+
export const OpenAiServiceTier = {
64+
Default: "default",
65+
Flex: "flex",
66+
Priority: "priority",
67+
} as const
68+
69+
export const serviceTiers = [OpenAiServiceTier.Default, OpenAiServiceTier.Flex, OpenAiServiceTier.Priority] as const
6170
export const serviceTierSchema = z.enum(serviceTiers)
6271
export type ServiceTier = z.infer<typeof serviceTierSchema>
6372

73+
/**
74+
* Service tiers for Codex requests authenticated through a ChatGPT subscription.
75+
*/
76+
export const OpenAiCodexServiceTier = {
77+
Default: "default",
78+
Priority: "priority",
79+
} as const
80+
81+
export const openAiCodexServiceTiers = [OpenAiCodexServiceTier.Default, OpenAiCodexServiceTier.Priority] as const
82+
export const openAiCodexServiceTierSchema = z.enum(openAiCodexServiceTiers)
83+
export type OpenAiCodexServiceTier = z.infer<typeof openAiCodexServiceTierSchema>
84+
6485
/**
6586
* ModelParameter
6687
*/

packages/types/src/provider-settings.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
import { z } from "zod"
22

3-
import { modelInfoSchema, reasoningEffortSettingSchema, verbosityLevelsSchema, serviceTierSchema } from "./model.js"
3+
import {
4+
modelInfoSchema,
5+
openAiCodexServiceTierSchema,
6+
reasoningEffortSettingSchema,
7+
verbosityLevelsSchema,
8+
serviceTierSchema,
9+
} from "./model.js"
410
import { codebaseIndexProviderSchema } from "./codebase-index.js"
511
import {
612
providerIdentifiers,
@@ -38,6 +44,7 @@ import {
3844
*/
3945

4046
export const DEFAULT_CONSECUTIVE_MISTAKE_LIMIT = 3
47+
export const OPEN_AI_CODEX_SERVICE_TIER_KEY = "openAiCodexServiceTier"
4148

4249
/**
4350
* DynamicProvider
@@ -279,7 +286,8 @@ const geminiCliSchema = apiModelIdProviderModelSchema.extend({
279286
})
280287

281288
const openAiCodexSchema = apiModelIdProviderModelSchema.extend({
282-
// No additional settings needed - uses OAuth authentication
289+
// Codex "Fast" mode maps to the Responses API priority service tier.
290+
[OPEN_AI_CODEX_SERVICE_TIER_KEY]: openAiCodexServiceTierSchema.optional(),
283291
})
284292

285293
const openAiNativeSchema = apiModelIdProviderModelSchema.extend({

src/api/providers/__tests__/bedrock.spec.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ import {
5858
BEDROCK_1M_CONTEXT_MODEL_IDS,
5959
BEDROCK_SERVICE_TIER_MODEL_IDS,
6060
bedrockModels,
61+
SERVICE_TIER_KEY,
6162
ApiProviderError,
6263
} from "@roo-code/types"
6364

@@ -1233,10 +1234,10 @@ describe("AwsBedrockHandler", () => {
12331234
const commandArg = mockConverseStreamCommand.mock.calls[0][0] as any
12341235

12351236
// service_tier should be at the top level of the payload
1236-
expect(commandArg.service_tier).toBe("PRIORITY")
1237+
expect(commandArg[SERVICE_TIER_KEY]).toBe("PRIORITY")
12371238
// service_tier should NOT be in additionalModelRequestFields
12381239
if (commandArg.additionalModelRequestFields) {
1239-
expect(commandArg.additionalModelRequestFields.service_tier).toBeUndefined()
1240+
expect(commandArg.additionalModelRequestFields[SERVICE_TIER_KEY]).toBeUndefined()
12401241
}
12411242
})
12421243

@@ -1263,10 +1264,10 @@ describe("AwsBedrockHandler", () => {
12631264
const commandArg = mockConverseStreamCommand.mock.calls[0][0] as any
12641265

12651266
// service_tier should be at the top level of the payload
1266-
expect(commandArg.service_tier).toBe("FLEX")
1267+
expect(commandArg[SERVICE_TIER_KEY]).toBe("FLEX")
12671268
// service_tier should NOT be in additionalModelRequestFields
12681269
if (commandArg.additionalModelRequestFields) {
1269-
expect(commandArg.additionalModelRequestFields.service_tier).toBeUndefined()
1270+
expect(commandArg.additionalModelRequestFields[SERVICE_TIER_KEY]).toBeUndefined()
12701271
}
12711272
})
12721273

@@ -1294,9 +1295,9 @@ describe("AwsBedrockHandler", () => {
12941295
const commandArg = mockConverseStreamCommand.mock.calls[0][0] as any
12951296

12961297
// Service tier should NOT be included for unsupported models (at top level or in additionalModelRequestFields)
1297-
expect(commandArg.service_tier).toBeUndefined()
1298+
expect(commandArg[SERVICE_TIER_KEY]).toBeUndefined()
12981299
if (commandArg.additionalModelRequestFields) {
1299-
expect(commandArg.additionalModelRequestFields.service_tier).toBeUndefined()
1300+
expect(commandArg.additionalModelRequestFields[SERVICE_TIER_KEY]).toBeUndefined()
13001301
}
13011302
})
13021303

@@ -1323,9 +1324,9 @@ describe("AwsBedrockHandler", () => {
13231324
const commandArg = mockConverseStreamCommand.mock.calls[0][0] as any
13241325

13251326
// Service tier should NOT be included when not specified (at top level or in additionalModelRequestFields)
1326-
expect(commandArg.service_tier).toBeUndefined()
1327+
expect(commandArg[SERVICE_TIER_KEY]).toBeUndefined()
13271328
if (commandArg.additionalModelRequestFields) {
1328-
expect(commandArg.additionalModelRequestFields.service_tier).toBeUndefined()
1329+
expect(commandArg.additionalModelRequestFields[SERVICE_TIER_KEY]).toBeUndefined()
13291330
}
13301331
})
13311332
})

0 commit comments

Comments
 (0)