Skip to content

Commit a1286cb

Browse files
WebMadWebMad
committed
lint(providers): enforce canonical identifiers
Add a focused ESLint rule for canonical providerIdentifiers usage, migrate remaining provider and embedding literals, and update production and test coverage.
1 parent 6d3d2dd commit a1286cb

48 files changed

Lines changed: 1057 additions & 655 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎pnpm-lock.yaml‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/__tests__/no-raw-provider-identifiers.spec.mjs‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
import { Linter } from "eslint"
2+
import typescriptParser from "@typescript-eslint/parser"
3+
import { describe, expect, it } from "vitest"
4+
5+
import { noRawProviderIdentifiers } from "../eslint-rules/no-raw-provider-identifiers.mjs"
6+
7+
const linter = new Linter({ configType: "eslintrc" })
8+
9+
linter.defineRule("zoo/no-raw-provider-identifiers", noRawProviderIdentifiers)
10+
linter.defineParser("@typescript-eslint/parser", typescriptParser)
11+
12+
function lint(code) {
13+
return linter.verify(code, {
14+
parserOptions: { ecmaVersion: 2022, sourceType: "module" },
15+
rules: { "zoo/no-raw-provider-identifiers": "error" },
16+
})
17+
}
18+
19+
function lintTypeScript(code) {
20+
return linter.verify(code, {
21+
parser: "@typescript-eslint/parser",
22+
parserOptions: {
23+
ecmaVersion: 2022,
24+
sourceType: "module",
25+
warnOnUnsupportedTypeScriptVersion: false,
26+
},
27+
rules: { "zoo/no-raw-provider-identifiers": "error" },
28+
})
29+
}
30+
31+
describe("no-raw-provider-identifiers", () => {
32+
it("rejects a canonical provider literal in an apiProvider property", () => {
33+
const messages = lint('const config = { apiProvider: "poe" }')
34+
35+
expect(messages).toHaveLength(1)
36+
expect(messages[0]).toMatchObject({
37+
ruleId: "zoo/no-raw-provider-identifiers",
38+
message: 'Use providerIdentifiers.poe instead of the raw provider identifier "poe".',
39+
})
40+
})
41+
42+
it("allows a non-canonical literal and a canonical registry member", () => {
43+
expect(lint('const config = { apiProvider: "external-provider" }')).toHaveLength(0)
44+
expect(lint("const config = { apiProvider: providerIdentifiers.poe }")).toHaveLength(0)
45+
})
46+
47+
it("matches provider-like property names and static template literals", () => {
48+
const messages = lint('const config = { provider: "poe", imageProvider: `openrouter` }')
49+
50+
expect(messages).toHaveLength(2)
51+
})
52+
53+
it("allows an empty static template in a provider-like context", () => {
54+
expect(lint("const config = { apiProvider: `` }")).toHaveLength(0)
55+
})
56+
57+
it("rejects canonical literals in provider-like variable declarations", () => {
58+
const messages = lint(`
59+
const apiProvider = "poe"
60+
let fallbackProvider = \`openrouter\`
61+
const label = "poe"
62+
`)
63+
64+
expect(messages.map(({ message }) => message)).toEqual([
65+
'Use providerIdentifiers.poe instead of the raw provider identifier "poe".',
66+
'Use providerIdentifiers.openrouter instead of the raw provider identifier "openrouter".',
67+
])
68+
})
69+
70+
it("rejects canonical provider literals wrapped in TypeScript expressions", () => {
71+
const messages = lintTypeScript(`
72+
const apiProvider = "poe" as ApiProvider
73+
const fallbackProvider = "openrouter" satisfies ApiProvider
74+
const imageProvider = <ApiProvider>"openai-native"
75+
const nestedProvider = ("anthropic" as ApiProvider)!
76+
`)
77+
78+
expect(messages.map(({ message }) => message)).toEqual([
79+
'Use providerIdentifiers.poe instead of the raw provider identifier "poe".',
80+
'Use providerIdentifiers.openrouter instead of the raw provider identifier "openrouter".',
81+
'Use providerIdentifiers.openaiNative instead of the raw provider identifier "openai-native".',
82+
'Use providerIdentifiers.anthropic instead of the raw provider identifier "anthropic".',
83+
])
84+
})
85+
86+
it("rejects canonical literals in provider-like assignments and comparisons", () => {
87+
const messages = lint(`
88+
config["apiProvider"] = "poe"
89+
if (imageProvider === "openrouter") {}
90+
if ("openai-native" !== config.fallbackProvider) {}
91+
`)
92+
93+
expect(messages.map(({ message }) => message)).toEqual([
94+
'Use providerIdentifiers.poe instead of the raw provider identifier "poe".',
95+
'Use providerIdentifiers.openrouter instead of the raw provider identifier "openrouter".',
96+
'Use providerIdentifiers.openaiNative instead of the raw provider identifier "openai-native".',
97+
])
98+
})
99+
100+
it("rejects canonical literals in provider-like switch cases", () => {
101+
const messages = lint(`
102+
switch (config.apiProvider) {
103+
case "poe": break
104+
case providerIdentifiers.openrouter: break
105+
}
106+
`)
107+
108+
expect(messages).toHaveLength(1)
109+
expect(messages[0].message).toContain("providerIdentifiers.poe")
110+
})
111+
112+
it("does not report canonical values outside provider-like contexts", () => {
113+
const messages = lint(`
114+
const label = "poe"
115+
const config = { protocol: "anthropic", format: "openai" }
116+
config[dynamicKey] = "poe"
117+
if (apiProtocol === "anthropic") {}
118+
if (provider > "poe") {}
119+
switch (format) { case "openai": break }
120+
`)
121+
122+
expect(messages).toHaveLength(0)
123+
})
124+
})

‎src/__tests__/single-open-invariant.spec.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { TaskScheduler } from "../core/task/TaskScheduler"
88
import { type Task } from "../core/task/Task"
99
import { API } from "../extension/api"
1010
import * as ProfileValidatorMod from "../shared/ProfileValidator"
11+
import { providerIdentifiers } from "@roo-code/types/provider-identifiers"
1112

1213
type PrivateClineProviderMethods = {
1314
createTask: (
@@ -45,7 +46,7 @@ vi.mock("../core/task/Task", () => {
4546
}) {
4647
this.taskId = opts.historyItem?.id ?? `task-${Math.random().toString(36).slice(2, 8)}`
4748
this.parentTask = opts.parentTask
48-
this.apiConfiguration = opts.apiConfiguration ?? { apiProvider: "anthropic" }
49+
this.apiConfiguration = opts.apiConfiguration ?? { apiProvider: providerIdentifiers.anthropic }
4950
opts.onCreated?.(this)
5051
}
5152
start() {}
@@ -86,7 +87,7 @@ describe("Single-open-task invariant", () => {
8687
},
8788
setValues: vi.fn(),
8889
getState: vi.fn().mockResolvedValue({
89-
apiConfiguration: { apiProvider: "anthropic", consecutiveMistakeLimit: 0 },
90+
apiConfiguration: { apiProvider: providerIdentifiers.anthropic, consecutiveMistakeLimit: 0 },
9091
organizationAllowList: "*",
9192
enableCheckpoints: true,
9293
checkpointTimeout: 60,
@@ -130,7 +131,7 @@ describe("Single-open-task invariant", () => {
130131
taskScheduler: new TaskScheduler(),
131132
setValues: vi.fn(),
132133
getState: vi.fn().mockResolvedValue({
133-
apiConfiguration: { apiProvider: "anthropic", consecutiveMistakeLimit: 0 },
134+
apiConfiguration: { apiProvider: providerIdentifiers.anthropic, consecutiveMistakeLimit: 0 },
134135
organizationAllowList: "*",
135136
enableCheckpoints: true,
136137
checkpointTimeout: 60,
@@ -182,7 +183,7 @@ describe("Single-open-task invariant", () => {
182183
listConfig: vi.fn().mockResolvedValue([]),
183184
},
184185
getState: vi.fn().mockResolvedValue({
185-
apiConfiguration: { apiProvider: "anthropic", consecutiveMistakeLimit: 0 },
186+
apiConfiguration: { apiProvider: providerIdentifiers.anthropic, consecutiveMistakeLimit: 0 },
186187
enableCheckpoints: true,
187188
checkpointTimeout: 60,
188189
experiments: {},
@@ -256,7 +257,7 @@ describe("Single-open-task invariant", () => {
256257
listConfig: vi.fn().mockResolvedValue([]),
257258
},
258259
getState: vi.fn().mockResolvedValue({
259-
apiConfiguration: { apiProvider: "anthropic", consecutiveMistakeLimit: 0 },
260+
apiConfiguration: { apiProvider: providerIdentifiers.anthropic, consecutiveMistakeLimit: 0 },
260261
enableCheckpoints: true,
261262
checkpointTimeout: 60,
262263
experiments: {},
@@ -328,7 +329,7 @@ describe("Single-open-task invariant", () => {
328329
listConfig: vi.fn().mockResolvedValue([]),
329330
},
330331
getState: vi.fn().mockResolvedValue({
331-
apiConfiguration: { apiProvider: "anthropic", consecutiveMistakeLimit: 0 },
332+
apiConfiguration: { apiProvider: providerIdentifiers.anthropic, consecutiveMistakeLimit: 0 },
332333
enableCheckpoints: true,
333334
checkpointTimeout: 60,
334335
experiments: {},

‎src/api/providers/__tests__/bedrock-reasoning.spec.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { BedrockRuntimeClient, ConverseStreamCommand } from "@aws-sdk/client-bed
55
import { logger } from "../../../utils/logging"
66

77
import { clearAllMocks } from "../../../test-utils/reset"
8+
import { providerIdentifiers } from "@roo-code/types/provider-identifiers"
89

910
// Mock the AWS SDK
1011
vi.mock("@aws-sdk/client-bedrock-runtime")
@@ -45,7 +46,7 @@ describe("AwsBedrockHandler - Extended Thinking", () => {
4546
describe("Extended Thinking Support", () => {
4647
it("should include thinking parameter for Claude Sonnet 4 when reasoning is enabled", async () => {
4748
handler = new AwsBedrockHandler({
48-
apiProvider: "bedrock",
49+
apiProvider: providerIdentifiers.bedrock,
4950
apiModelId: "anthropic.claude-sonnet-4-20250514-v1:0",
5051
awsRegion: "us-east-1",
5152
enableReasoningEffort: true,
@@ -113,7 +114,7 @@ describe("AwsBedrockHandler - Extended Thinking", () => {
113114

114115
it("should pass thinking parameters from metadata", async () => {
115116
handler = new AwsBedrockHandler({
116-
apiProvider: "bedrock",
117+
apiProvider: providerIdentifiers.bedrock,
117118
apiModelId: "anthropic.claude-3-7-sonnet-20250219-v1:0",
118119
awsRegion: "us-east-1",
119120
})
@@ -156,7 +157,7 @@ describe("AwsBedrockHandler - Extended Thinking", () => {
156157

157158
it("should log when extended thinking is enabled", async () => {
158159
handler = new AwsBedrockHandler({
159-
apiProvider: "bedrock",
160+
apiProvider: providerIdentifiers.bedrock,
160161
apiModelId: "anthropic.claude-opus-4-20250514-v1:0",
161162
awsRegion: "us-east-1",
162163
enableReasoningEffort: true,
@@ -188,7 +189,7 @@ describe("AwsBedrockHandler - Extended Thinking", () => {
188189

189190
it("should not include topP when thinking is disabled (global removal)", async () => {
190191
handler = new AwsBedrockHandler({
191-
apiProvider: "bedrock",
192+
apiProvider: providerIdentifiers.bedrock,
192193
apiModelId: "anthropic.claude-3-7-sonnet-20250219-v1:0",
193194
awsRegion: "us-east-1",
194195
// Note: no enableReasoningEffort = true, so thinking is disabled
@@ -234,7 +235,7 @@ describe("AwsBedrockHandler - Extended Thinking", () => {
234235

235236
it("should enable reasoning when enableReasoningEffort is true in settings", async () => {
236237
handler = new AwsBedrockHandler({
237-
apiProvider: "bedrock",
238+
apiProvider: providerIdentifiers.bedrock,
238239
apiModelId: "anthropic.claude-sonnet-4-20250514-v1:0",
239240
awsRegion: "us-east-1",
240241
enableReasoningEffort: true, // This should trigger reasoning
@@ -288,7 +289,7 @@ describe("AwsBedrockHandler - Extended Thinking", () => {
288289

289290
it("should support API key authentication", async () => {
290291
handler = new AwsBedrockHandler({
291-
apiProvider: "bedrock",
292+
apiProvider: providerIdentifiers.bedrock,
292293
apiModelId: "anthropic.claude-3-5-sonnet-20241022-v2:0",
293294
awsRegion: "us-east-1",
294295
awsUseApiKey: true,

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { getModelMaxOutputTokens } from "../../../shared/api"
1010
import { FriendliHandler } from "../friendli"
1111
import { asyncStreamFrom, collectStream } from "../../../test-utils/stream"
1212
import { clearAllMocks } from "../../../test-utils/reset"
13+
import { providerIdentifiers } from "@roo-code/types/provider-identifiers"
1314

1415
// Create mock functions
1516
const mockCreate = vi.fn()
@@ -324,7 +325,7 @@ describe("FriendliHandler", () => {
324325

325326
describe("buildApiHandler friendli wiring", () => {
326327
it("returns a FriendliHandler for apiProvider='friendli'", () => {
327-
const handler = buildApiHandler({ apiProvider: "friendli", friendliApiKey: "test-key" })
328+
const handler = buildApiHandler({ apiProvider: providerIdentifiers.friendli, friendliApiKey: "test-key" })
328329
expect(handler).toBeInstanceOf(FriendliHandler)
329330
})
330331
})
@@ -335,7 +336,7 @@ describe("Friendli model max output tokens (clamping behavior)", () => {
335336
const result = getModelMaxOutputTokens({
336337
modelId: "zai-org/GLM-5.2",
337338
model,
338-
settings: { apiProvider: "friendli" },
339+
settings: { apiProvider: providerIdentifiers.friendli },
339340
format: "openai",
340341
})
341342
// 1_000_000 * 0.2 = 200_000 > 131_072 → no clamping
@@ -347,7 +348,7 @@ describe("Friendli model max output tokens (clamping behavior)", () => {
347348
const result = getModelMaxOutputTokens({
348349
modelId: "zai-org/GLM-5.1",
349350
model,
350-
settings: { apiProvider: "friendli" },
351+
settings: { apiProvider: providerIdentifiers.friendli },
351352
format: "openai",
352353
})
353354
// 200_000 * 0.2 = 40_000 < 131_072 → clamped to 40_000
@@ -359,7 +360,7 @@ describe("Friendli model max output tokens (clamping behavior)", () => {
359360
const result = getModelMaxOutputTokens({
360361
modelId: "zai-org/GLM-5.1",
361362
model,
362-
settings: { apiProvider: "friendli", modelMaxTokens: 80_000 },
363+
settings: { apiProvider: providerIdentifiers.friendli, modelMaxTokens: 80_000 },
363364
format: "openai",
364365
})
365366
// supportsMaxTokens=true, user set 80k, model ceiling 131072 → min(80000, 131072) = 80000

0 commit comments

Comments
 (0)