Skip to content

Commit b736800

Browse files
roomoteroomote
committed
fix: normalize OpenAI custom URLs to prevent /v1 duplication
- Add normalizeBaseUrl method to OpenAiHandler to handle URL normalization - Ensure /v1 is present but not duplicated for OpenAI-compatible APIs - Skip normalization for Azure endpoints - Update getOpenAiModels function with same normalization logic - Add comprehensive tests for URL normalization scenarios Fixes #8282
1 parent 8dbd8c4 commit b736800

File tree

2 files changed

+227
-49
lines changed

2 files changed

+227
-49
lines changed

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

Lines changed: 167 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,43 @@ const mockCreate = vitest.fn()
1212

1313
vitest.mock("openai", () => {
1414
const mockConstructor = vitest.fn()
15-
return {
16-
__esModule: true,
17-
default: mockConstructor.mockImplementation(() => ({
18-
chat: {
19-
completions: {
20-
create: mockCreate.mockImplementation(async (options) => {
21-
if (!options.stream) {
22-
return {
23-
id: "test-completion",
15+
const mockImplementation = () => ({
16+
chat: {
17+
completions: {
18+
create: mockCreate.mockImplementation(async (options) => {
19+
if (!options.stream) {
20+
return {
21+
id: "test-completion",
22+
choices: [
23+
{
24+
message: { role: "assistant", content: "Test response", refusal: null },
25+
finish_reason: "stop",
26+
index: 0,
27+
},
28+
],
29+
usage: {
30+
prompt_tokens: 10,
31+
completion_tokens: 5,
32+
total_tokens: 15,
33+
},
34+
}
35+
}
36+
37+
return {
38+
[Symbol.asyncIterator]: async function* () {
39+
yield {
40+
choices: [
41+
{
42+
delta: { content: "Test response" },
43+
index: 0,
44+
},
45+
],
46+
usage: null,
47+
}
48+
yield {
2449
choices: [
2550
{
26-
message: { role: "assistant", content: "Test response", refusal: null },
27-
finish_reason: "stop",
51+
delta: {},
2852
index: 0,
2953
},
3054
],
@@ -34,38 +58,17 @@ vitest.mock("openai", () => {
3458
total_tokens: 15,
3559
},
3660
}
37-
}
38-
39-
return {
40-
[Symbol.asyncIterator]: async function* () {
41-
yield {
42-
choices: [
43-
{
44-
delta: { content: "Test response" },
45-
index: 0,
46-
},
47-
],
48-
usage: null,
49-
}
50-
yield {
51-
choices: [
52-
{
53-
delta: {},
54-
index: 0,
55-
},
56-
],
57-
usage: {
58-
prompt_tokens: 10,
59-
completion_tokens: 5,
60-
total_tokens: 15,
61-
},
62-
}
63-
},
64-
}
65-
}),
66-
},
61+
},
62+
}
63+
}),
6764
},
68-
})),
65+
},
66+
})
67+
68+
return {
69+
__esModule: true,
70+
default: mockConstructor.mockImplementation(mockImplementation),
71+
AzureOpenAI: mockConstructor.mockImplementation(mockImplementation),
6972
}
7073
})
7174

@@ -105,6 +108,50 @@ describe("OpenAiHandler", () => {
105108
expect(handlerWithCustomUrl).toBeInstanceOf(OpenAiHandler)
106109
})
107110

111+
it("should normalize base URL to prevent /v1 duplication", () => {
112+
// Test URL that already ends with /v1
113+
const urlWithV1 = "https://custom.openai.com/v1"
114+
const handler1 = new OpenAiHandler({
115+
...mockOptions,
116+
openAiBaseUrl: urlWithV1,
117+
})
118+
expect(handler1).toBeInstanceOf(OpenAiHandler)
119+
120+
// Test URL without /v1 (should add it)
121+
const urlWithoutV1 = "https://custom.openai.com"
122+
const handler2 = new OpenAiHandler({
123+
...mockOptions,
124+
openAiBaseUrl: urlWithoutV1,
125+
})
126+
expect(handler2).toBeInstanceOf(OpenAiHandler)
127+
128+
// Test URL with trailing slash (should add /v1)
129+
const urlWithTrailingSlash = "https://custom.openai.com/"
130+
const handler3 = new OpenAiHandler({
131+
...mockOptions,
132+
openAiBaseUrl: urlWithTrailingSlash,
133+
})
134+
expect(handler3).toBeInstanceOf(OpenAiHandler)
135+
})
136+
137+
it("should not modify Azure endpoints", () => {
138+
// Test Azure OpenAI endpoint
139+
const azureUrl = "https://myinstance.openai.azure.com/openai/deployments/mymodel"
140+
const azureHandler = new OpenAiHandler({
141+
...mockOptions,
142+
openAiBaseUrl: azureUrl,
143+
})
144+
expect(azureHandler).toBeInstanceOf(OpenAiHandler)
145+
146+
// Test Azure AI Inference Service endpoint
147+
const azureAiUrl = "https://myinstance.services.ai.azure.com"
148+
const azureAiHandler = new OpenAiHandler({
149+
...mockOptions,
150+
openAiBaseUrl: azureAiUrl,
151+
})
152+
expect(azureAiHandler).toBeInstanceOf(OpenAiHandler)
153+
})
154+
108155
it("should set default headers correctly", () => {
109156
// Check that the OpenAI constructor was called with correct parameters
110157
expect(vi.mocked(OpenAI)).toHaveBeenCalledWith({
@@ -831,6 +878,84 @@ describe("getOpenAiModels", () => {
831878
expect(result).toEqual(["model-1", "model-2"])
832879
})
833880

881+
it("should normalize URLs to prevent /v1 duplication", async () => {
882+
const mockResponse = {
883+
data: {
884+
data: [{ id: "model-1" }],
885+
},
886+
}
887+
vi.mocked(axios.get).mockResolvedValueOnce(mockResponse)
888+
889+
// URL already ending with /v1 should not get another /v1
890+
const result = await getOpenAiModels("https://custom.api.com/v1", "test-key")
891+
892+
expect(axios.get).toHaveBeenCalledWith("https://custom.api.com/v1/models", expect.any(Object))
893+
expect(result).toEqual(["model-1"])
894+
})
895+
896+
it("should add /v1 to URLs that don't have it", async () => {
897+
const mockResponse = {
898+
data: {
899+
data: [{ id: "model-1" }],
900+
},
901+
}
902+
vi.mocked(axios.get).mockResolvedValueOnce(mockResponse)
903+
904+
// URL without /v1 should get /v1 added
905+
const result = await getOpenAiModels("https://custom.api.com", "test-key")
906+
907+
expect(axios.get).toHaveBeenCalledWith("https://custom.api.com/v1/models", expect.any(Object))
908+
expect(result).toEqual(["model-1"])
909+
})
910+
911+
it("should handle URLs with trailing slash correctly", async () => {
912+
const mockResponse = {
913+
data: {
914+
data: [{ id: "model-1" }],
915+
},
916+
}
917+
vi.mocked(axios.get).mockResolvedValueOnce(mockResponse)
918+
919+
// URL with trailing slash should get /v1 added correctly
920+
const result = await getOpenAiModels("https://custom.api.com/", "test-key")
921+
922+
expect(axios.get).toHaveBeenCalledWith("https://custom.api.com/v1/models", expect.any(Object))
923+
expect(result).toEqual(["model-1"])
924+
})
925+
926+
it("should not modify Azure endpoints", async () => {
927+
const mockResponse = {
928+
data: {
929+
data: [{ id: "azure-model" }],
930+
},
931+
}
932+
vi.mocked(axios.get).mockResolvedValueOnce(mockResponse)
933+
934+
// Azure endpoint should not be modified
935+
const result = await getOpenAiModels("https://myinstance.openai.azure.com/openai/deployments", "test-key")
936+
937+
expect(axios.get).toHaveBeenCalledWith(
938+
"https://myinstance.openai.azure.com/openai/deployments/models",
939+
expect.any(Object),
940+
)
941+
expect(result).toEqual(["azure-model"])
942+
})
943+
944+
it("should not modify Azure AI Inference Service endpoints", async () => {
945+
const mockResponse = {
946+
data: {
947+
data: [{ id: "azure-ai-model" }],
948+
},
949+
}
950+
vi.mocked(axios.get).mockResolvedValueOnce(mockResponse)
951+
952+
// Azure AI Inference Service endpoint should not be modified
953+
const result = await getOpenAiModels("https://myinstance.services.ai.azure.com", "test-key")
954+
955+
expect(axios.get).toHaveBeenCalledWith("https://myinstance.services.ai.azure.com/models", expect.any(Object))
956+
expect(result).toEqual(["azure-ai-model"])
957+
})
958+
834959
it("should handle baseUrl with leading spaces", async () => {
835960
const mockResponse = {
836961
data: {

‎src/api/providers/openai.ts‎

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl
3838
super()
3939
this.options = options
4040

41-
const baseURL = this.options.openAiBaseUrl ?? "https://api.openai.com/v1"
41+
const baseURL = this.normalizeBaseUrl(this.options.openAiBaseUrl ?? "https://api.openai.com/v1")
4242
const apiKey = this.options.openAiApiKey ?? "not-provided"
43-
const isAzureAiInference = this._isAzureAiInference(this.options.openAiBaseUrl)
44-
const urlHost = this._getUrlHost(this.options.openAiBaseUrl)
43+
const isAzureAiInference = this._isAzureAiInference(baseURL)
44+
const urlHost = this._getUrlHost(baseURL)
4545
const isAzureOpenAi = urlHost === "azure.com" || urlHost.endsWith(".azure.com") || options.openAiUseAzure
4646

4747
const headers = {
@@ -423,6 +423,41 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl
423423
return urlHost.endsWith(".services.ai.azure.com")
424424
}
425425

426+
/**
427+
* Normalizes the base URL to ensure it ends with /v1 for OpenAI-compatible APIs
428+
* but doesn't duplicate /v1 if it's already present
429+
*/
430+
private normalizeBaseUrl(baseUrl: string): string {
431+
// Trim whitespace
432+
let normalizedUrl = baseUrl.trim()
433+
434+
// For standard OpenAI API, keep as-is
435+
if (normalizedUrl === "https://api.openai.com/v1") {
436+
return normalizedUrl
437+
}
438+
439+
// For Azure endpoints, don't modify
440+
const urlHost = this._getUrlHost(normalizedUrl)
441+
if (urlHost.endsWith(".azure.com") || urlHost.endsWith(".services.ai.azure.com")) {
442+
return normalizedUrl
443+
}
444+
445+
// For other OpenAI-compatible APIs, ensure /v1 is present but not duplicated
446+
// The OpenAI SDK expects the base URL to include /v1 for compatibility
447+
if (!normalizedUrl.endsWith("/v1")) {
448+
// Remove trailing slash if present
449+
if (normalizedUrl.endsWith("/")) {
450+
normalizedUrl = normalizedUrl.slice(0, -1)
451+
}
452+
// Only add /v1 if it's not already there
453+
if (!normalizedUrl.endsWith("/v1")) {
454+
normalizedUrl = `${normalizedUrl}/v1`
455+
}
456+
}
457+
458+
return normalizedUrl
459+
}
460+
426461
/**
427462
* Adds max_completion_tokens to the request body if needed based on provider configuration
428463
* Note: max_tokens is deprecated in favor of max_completion_tokens as per OpenAI documentation
@@ -449,13 +484,31 @@ export async function getOpenAiModels(baseUrl?: string, apiKey?: string, openAiH
449484
return []
450485
}
451486

452-
// Trim whitespace from baseUrl to handle cases where users accidentally include spaces
453-
const trimmedBaseUrl = baseUrl.trim()
487+
// Normalize the base URL using the same logic as the OpenAiHandler
488+
let normalizedUrl = baseUrl.trim()
454489

455-
if (!URL.canParse(trimmedBaseUrl)) {
490+
if (!URL.canParse(normalizedUrl)) {
456491
return []
457492
}
458493

494+
// For Azure endpoints, don't modify
495+
const urlHost = new URL(normalizedUrl).host
496+
const isAzure = urlHost.endsWith(".azure.com") || urlHost.endsWith(".services.ai.azure.com")
497+
498+
if (!isAzure && normalizedUrl !== "https://api.openai.com/v1") {
499+
// For other OpenAI-compatible APIs, ensure /v1 is present but not duplicated
500+
if (!normalizedUrl.endsWith("/v1")) {
501+
// Remove trailing slash if present
502+
if (normalizedUrl.endsWith("/")) {
503+
normalizedUrl = normalizedUrl.slice(0, -1)
504+
}
505+
// Only add /v1 if it's not already there
506+
if (!normalizedUrl.endsWith("/v1")) {
507+
normalizedUrl = `${normalizedUrl}/v1`
508+
}
509+
}
510+
}
511+
459512
const config: Record<string, any> = {}
460513
const headers: Record<string, string> = {
461514
...DEFAULT_HEADERS,
@@ -470,7 +523,7 @@ export async function getOpenAiModels(baseUrl?: string, apiKey?: string, openAiH
470523
config["headers"] = headers
471524
}
472525

473-
const response = await axios.get(`${trimmedBaseUrl}/models`, config)
526+
const response = await axios.get(`${normalizedUrl}/models`, config)
474527
const modelsArray = response.data?.data?.map((model: any) => model.id) || []
475528
return [...new Set<string>(modelsArray)]
476529
} catch (error) {

0 commit comments

Comments
 (0)