Skip to content

Commit 92ee5a3

Browse files
committed
fix(openrouter): read the cache capability from the live model catalog
What: - Gate the Claude cache-breakpoint widening on the model-level catalog record (`this.models[modelId]`) instead of the resolved `model.info`, and require the id to have actually resolved from the fetched catalog. - Qualify the `session_id` comment: OpenRouter does not use sticky routing when a manual `provider.order` is set, so pinning a provider limits the benefit. - Add `google/gemini-9-pro` to the mocked catalog so the Gemini negative control states its premise instead of leaning on the hardcoded default info's flag. - Extract the repeated OpenAI `chat.completions.create` stub into one test helper. Why: Round-1 review found the widened gate fails open in two ways. `model.info` is not a live capability source: on an empty or degraded catalog it is `openRouterDefaultModelInfo`, which hardcodes `supportsPromptCache: true`, so any `anthropic/*` id gained breakpoints the base allowlist withheld. With a pinned `openRouterSpecificProvider`, `getModel()` substitutes that endpoint's record, whose flag comes from one endpoint's pricing, so the same Claude model gained or lost breakpoints depending on pinning and the fix silently disappeared. Impact: Behavior is unchanged for allowlisted ids and for every non-Anthropic id. A non-allowlisted `anthropic/*` id now needs a `supportsPromptCache: true` record from the fetched catalog, which is exactly what the live flag meant to say.
1 parent 611e82a commit 92ee5a3

2 files changed

Lines changed: 183 additions & 12 deletions

File tree

‎src/api/providers/__tests__/openrouter.spec.ts‎

Lines changed: 168 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import type { ModelInfo } from "@roo-code/types"
2323
import { applyOpenRouterMoonshotK3Profile } from "../fetchers/openrouter"
2424

2525
import { OpenRouterHandler } from "../openrouter"
26+
import { getModels } from "../fetchers/modelCache"
2627
import { Package } from "../../../shared/package"
2728
import { makeApiHandlerOptions } from "../../../test-utils/api"
2829
import { asyncStreamFrom, collectStream } from "../../../test-utils/stream"
@@ -95,6 +96,20 @@ vitest.mock("../fetchers/modelCache", () => ({
9596
description: "Claude 4.8 Opus",
9697
thinking: false,
9798
},
99+
// Not in OPEN_ROUTER_PROMPT_CACHING_MODELS and not Anthropic: the
100+
// negative control for the widened gate. It advertises cache pricing
101+
// so the fixture states the premise instead of relying on the
102+
// hardcoded default info's flag.
103+
"google/gemini-9-pro": {
104+
maxTokens: 65536,
105+
contextWindow: 1000000,
106+
supportsImages: true,
107+
supportsPromptCache: true,
108+
inputPrice: 1.25,
109+
outputPrice: 10,
110+
cacheReadsPrice: 0.31,
111+
description: "Gemini 9 Pro (hypothetical)",
112+
},
98113
"anthropic/claude-3.7-sonnet:thinking": {
99114
maxTokens: 128000,
100115
contextWindow: 200000,
@@ -153,6 +168,32 @@ describe("OpenRouterHandler", () => {
153168
openRouterModelId: "anthropic/claude-sonnet-4",
154169
})
155170

171+
/**
172+
* Install a `chat.completions.create` stub on the vitest-mocked OpenAI class
173+
* and hand back the mock plus a restore function. The mocked class is
174+
* structurally incompatible with the narrow chat stub, so the double
175+
* assertion routes through `unknown` (instead of `any`) to keep this file's
176+
* no-explicit-any budget flat.
177+
*/
178+
function stubChatCompletions() {
179+
const create = vitest
180+
.fn()
181+
.mockResolvedValue(asyncStreamFrom([{ id: "test-id", choices: [{ delta: { content: "test response" } }] }]))
182+
const openAiPrototype = OpenAI as unknown as { prototype: { chat?: unknown } }
183+
const originalChat = openAiPrototype.prototype.chat
184+
openAiPrototype.prototype.chat = { completions: { create } }
185+
186+
return {
187+
create,
188+
requestParams: () => (create.mock.calls[0] as [Record<string, unknown>])[0],
189+
restore: () => {
190+
// clearAllMocks does not undo prototype assignment, so every stub
191+
// must be removed explicitly or it leaks into later tests.
192+
openAiPrototype.prototype.chat = originalChat
193+
},
194+
}
195+
}
196+
156197
beforeEach(() => {
157198
clearAllMocks()
158199
// Endpoint records default to "not fetched" (same as the real guard for
@@ -536,29 +577,147 @@ describe("OpenRouterHandler", () => {
536577
it("keeps Gemini cache breakpoints allowlist-only", async () => {
537578
// A Gemini model absent from the allowlist must not gain Anthropic-style
538579
// breakpoints just because its catalog record advertises cache pricing.
580+
// `google/gemini-9-pro` is in the mocked catalog with
581+
// `supportsPromptCache: true`, so the premise is stated by the fixture
582+
// rather than by the hardcoded default info's flag.
539583
const handler = new OpenRouterHandler(
540584
makeApiHandlerOptions({
541585
...mockOptions,
542586
openRouterModelId: "google/gemini-9-pro",
543587
}),
544588
)
589+
const stub = stubChatCompletions()
545590

546-
const mockStream = asyncStreamFrom([{ id: "test-id", choices: [{ delta: { content: "test response" } }] }])
547-
const mockCreate = vitest.fn().mockResolvedValue(mockStream)
548-
const chatStub = { completions: { create: mockCreate } }
549-
const openAiPrototype = OpenAI as unknown as { prototype: { chat?: unknown } }
550-
const originalChat = openAiPrototype.prototype.chat
551-
openAiPrototype.prototype.chat = chatStub
591+
try {
592+
await handler.createMessage("test system", [{ role: "user", content: "test message" }]).next()
593+
594+
expect(stub.requestParams()).toMatchObject({
595+
messages: [{ role: "system", content: "test system" }, expect.anything()],
596+
})
597+
} finally {
598+
stub.restore()
599+
}
600+
})
601+
602+
it("adds no cache breakpoints when the catalog is empty and the id is not allowlisted", async () => {
603+
// The fallback info hardcodes `supportsPromptCache: true`, so trusting
604+
// `model.info` would hand breakpoints to any `anthropic/*` id whenever
605+
// the catalog is empty or degraded. An id that never resolved from the
606+
// fetched catalog is not live evidence, so the allowlist alone decides.
607+
vitest.mocked(getModels).mockResolvedValueOnce({})
608+
609+
const handler = new OpenRouterHandler(
610+
makeApiHandlerOptions({
611+
...mockOptions,
612+
openRouterModelId: "anthropic/claude-not-in-catalog",
613+
}),
614+
)
615+
const stub = stubChatCompletions()
552616

553617
try {
554618
await handler.createMessage("test system", [{ role: "user", content: "test message" }]).next()
555619

556-
const [requestParams] = mockCreate.mock.calls[0] as [Record<string, unknown>]
557-
expect(requestParams).toMatchObject({
620+
expect(stub.requestParams()).toMatchObject({
558621
messages: [{ role: "system", content: "test system" }, expect.anything()],
559622
})
560623
} finally {
561-
openAiPrototype.prototype.chat = originalChat
624+
stub.restore()
625+
}
626+
})
627+
628+
it("reads the cache capability from the model record when a provider is pinned", async () => {
629+
// getModel() substitutes the pinned endpoint's record, whose
630+
// `supportsPromptCache` comes from that one endpoint's pricing. The gate
631+
// must follow the model-level catalog record instead, or pinning a
632+
// provider silently drops breakpoints for a caching Claude model.
633+
mockGetModelEndpoints.mockResolvedValue({
634+
anthropic: {
635+
maxTokens: 128000,
636+
contextWindow: 1000000,
637+
supportsImages: true,
638+
supportsPromptCache: false,
639+
inputPrice: 5,
640+
outputPrice: 25,
641+
description: "Claude 4.8 Opus (Anthropic endpoint)",
642+
},
643+
})
644+
645+
const handler = new OpenRouterHandler(
646+
makeApiHandlerOptions({
647+
...mockOptions,
648+
openRouterModelId: "anthropic/claude-opus-4.8",
649+
openRouterSpecificProvider: providerIdentifiers.anthropic,
650+
}),
651+
)
652+
const stub = stubChatCompletions()
653+
654+
try {
655+
await handler.createMessage("test system", [{ role: "user", content: "test message" }]).next()
656+
657+
// The endpoint record is what the request is built from...
658+
expect(stub.requestParams()).toMatchObject({ model: "anthropic/claude-opus-4.8" })
659+
// ...but the model-level flag still turns breakpoints on.
660+
expect(stub.requestParams()).toMatchObject({
661+
messages: expect.arrayContaining([
662+
expect.objectContaining({
663+
role: "system",
664+
content: expect.arrayContaining([
665+
expect.objectContaining({ cache_control: { type: "ephemeral" } }),
666+
]),
667+
}),
668+
]),
669+
})
670+
} finally {
671+
stub.restore()
672+
}
673+
})
674+
675+
it("adds no cache breakpoints when a pinned endpoint advertises caching the model record denies", async () => {
676+
// Mirror of the case above: an endpoint record that reports cache
677+
// pricing must not widen the gate for a model whose catalog record says
678+
// no. Otherwise pinning a provider would add breakpoints the unpinned
679+
// path withholds.
680+
mockGetModelEndpoints.mockResolvedValue({
681+
anthropic: {
682+
maxTokens: 128000,
683+
contextWindow: 1000000,
684+
supportsImages: true,
685+
supportsPromptCache: true,
686+
inputPrice: 5,
687+
outputPrice: 25,
688+
cacheReadsPrice: 0.5,
689+
description: "Claude 4.8 Opus (Anthropic endpoint)",
690+
},
691+
})
692+
vitest.mocked(getModels).mockResolvedValueOnce({
693+
"anthropic/claude-opus-4.8": {
694+
maxTokens: 128000,
695+
contextWindow: 1000000,
696+
supportsImages: true,
697+
supportsPromptCache: false,
698+
inputPrice: 5,
699+
outputPrice: 25,
700+
description: "Claude 4.8 Opus",
701+
},
702+
})
703+
704+
const handler = new OpenRouterHandler(
705+
makeApiHandlerOptions({
706+
...mockOptions,
707+
openRouterModelId: "anthropic/claude-opus-4.8",
708+
openRouterSpecificProvider: providerIdentifiers.anthropic,
709+
}),
710+
)
711+
const stub = stubChatCompletions()
712+
713+
try {
714+
await handler.createMessage("test system", [{ role: "user", content: "test message" }]).next()
715+
716+
expect(stub.requestParams()).toMatchObject({
717+
messages: [{ role: "system", content: "test system" }, expect.anything()],
718+
})
719+
} finally {
720+
stub.restore()
562721
}
563722
})
564723

‎src/api/providers/openrouter.ts‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -322,9 +322,18 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH
322322
// the allowlist as the fallback; Gemini and every other provider keep
323323
// allowlist-only behavior because breakpoint placement is not
324324
// interchangeable between them.
325+
//
326+
// Only the model-level catalog record carries a live flag, so read it from
327+
// `this.models` instead of `model.info`. `model.info` is not a safe source:
328+
// an empty catalog falls back to `openRouterDefaultModelInfo`, which
329+
// hardcodes `supportsPromptCache: true`, and a pinned provider substitutes
330+
// that endpoint's record, whose flag comes from one endpoint's pricing.
331+
// Either would hand breakpoints to every `anthropic/*` id, so an id that
332+
// did not resolve from the fetched catalog counts as no live evidence.
333+
const catalogInfo = Object.hasOwn(this.models, modelId) ? this.models[modelId] : undefined
325334
const supportsCacheBreakpoints =
326335
OPEN_ROUTER_PROMPT_CACHING_MODELS.has(modelId) ||
327-
(modelId.startsWith("anthropic/") && model.info.supportsPromptCache)
336+
(modelId.startsWith("anthropic/") && catalogInfo?.supportsPromptCache === true)
328337

329338
if (supportsCacheBreakpoints) {
330339
if (modelId.startsWith("google")) {
@@ -339,8 +348,11 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH
339348
model: modelId,
340349
// OpenRouter derives its sticky-routing key from the opening messages
341350
// unless `session_id` is sent, and without it stickiness only starts
342-
// after a cache hit is observed. A task ID pins every turn of the task
343-
// to the endpoint holding its warm cache from the first request.
351+
// after a cache hit is observed. A task ID gives every turn of the task
352+
// one routing key from the first request, so turns stay on the endpoint
353+
// holding the warm cache. OpenRouter does not use sticky routing when a
354+
// manual `provider.order` is set, so pinning a specific provider limits
355+
// the benefit to that provider's own cache.
344356
...(sessionId ? { session_id: sessionId } : {}),
345357
...(maxTokens && maxTokens > 0 && { max_tokens: maxTokens }),
346358
temperature,

0 commit comments

Comments
 (0)