mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
fix: normalize OpenAI custom base URLs with or without /v1 suffix
- Add normalizeOpenAiBaseUrl helper function to handle URL normalization - Automatically append /v1 to custom OpenAI-compatible base URLs when missing - Skip /v1 suffix for Azure endpoints which have different path structures - Update both OpenAiHandler constructor and getOpenAiModels function - Add comprehensive tests for URL normalization scenarios Fixes #9426
This commit is contained in:
parent
4ae0fc5f02
commit
4cb6444b0f
2 changed files with 239 additions and 57 deletions
|
|
@ -12,63 +12,79 @@ const mockCreate = vitest.fn()
|
|||
|
||||
vitest.mock("openai", () => {
|
||||
const mockConstructor = vitest.fn()
|
||||
const azureMockConstructor = vitest.fn()
|
||||
|
||||
const mockImplementation = () => ({
|
||||
chat: {
|
||||
completions: {
|
||||
create: mockCreate,
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
mockConstructor.mockImplementation(mockImplementation)
|
||||
azureMockConstructor.mockImplementation(mockImplementation)
|
||||
|
||||
// Store reference for later use in tests
|
||||
;(globalThis as any).__mockAzureOpenAI = azureMockConstructor
|
||||
|
||||
return {
|
||||
__esModule: true,
|
||||
default: mockConstructor.mockImplementation(() => ({
|
||||
chat: {
|
||||
completions: {
|
||||
create: mockCreate.mockImplementation(async (options) => {
|
||||
if (!options.stream) {
|
||||
return {
|
||||
id: "test-completion",
|
||||
choices: [
|
||||
{
|
||||
message: { role: "assistant", content: "Test response", refusal: null },
|
||||
finish_reason: "stop",
|
||||
index: 0,
|
||||
},
|
||||
],
|
||||
usage: {
|
||||
prompt_tokens: 10,
|
||||
completion_tokens: 5,
|
||||
total_tokens: 15,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
[Symbol.asyncIterator]: async function* () {
|
||||
yield {
|
||||
choices: [
|
||||
{
|
||||
delta: { content: "Test response" },
|
||||
index: 0,
|
||||
},
|
||||
],
|
||||
usage: null,
|
||||
}
|
||||
yield {
|
||||
choices: [
|
||||
{
|
||||
delta: {},
|
||||
index: 0,
|
||||
},
|
||||
],
|
||||
usage: {
|
||||
prompt_tokens: 10,
|
||||
completion_tokens: 5,
|
||||
total_tokens: 15,
|
||||
},
|
||||
}
|
||||
},
|
||||
}
|
||||
}),
|
||||
},
|
||||
},
|
||||
})),
|
||||
default: mockConstructor,
|
||||
AzureOpenAI: azureMockConstructor,
|
||||
}
|
||||
})
|
||||
|
||||
// Setup mockCreate default implementation
|
||||
beforeEach(() => {
|
||||
mockCreate.mockImplementation(async (options) => {
|
||||
if (!options.stream) {
|
||||
return {
|
||||
id: "test-completion",
|
||||
choices: [
|
||||
{
|
||||
message: { role: "assistant", content: "Test response", refusal: null },
|
||||
finish_reason: "stop",
|
||||
index: 0,
|
||||
},
|
||||
],
|
||||
usage: {
|
||||
prompt_tokens: 10,
|
||||
completion_tokens: 5,
|
||||
total_tokens: 15,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
[Symbol.asyncIterator]: async function* () {
|
||||
yield {
|
||||
choices: [
|
||||
{
|
||||
delta: { content: "Test response" },
|
||||
index: 0,
|
||||
},
|
||||
],
|
||||
usage: null,
|
||||
}
|
||||
yield {
|
||||
choices: [
|
||||
{
|
||||
delta: {},
|
||||
index: 0,
|
||||
},
|
||||
],
|
||||
usage: {
|
||||
prompt_tokens: 10,
|
||||
completion_tokens: 5,
|
||||
total_tokens: 15,
|
||||
},
|
||||
}
|
||||
},
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
// Mock axios for getOpenAiModels tests
|
||||
vitest.mock("axios", () => ({
|
||||
default: {
|
||||
|
|
@ -105,6 +121,84 @@ describe("OpenAiHandler", () => {
|
|||
expect(handlerWithCustomUrl).toBeInstanceOf(OpenAiHandler)
|
||||
})
|
||||
|
||||
it("should normalize base URLs without /v1 suffix", () => {
|
||||
const customBaseUrl = "https://custom.openai.com"
|
||||
const handlerWithCustomUrl = new OpenAiHandler({
|
||||
...mockOptions,
|
||||
openAiBaseUrl: customBaseUrl,
|
||||
})
|
||||
expect(handlerWithCustomUrl).toBeInstanceOf(OpenAiHandler)
|
||||
// Verify the OpenAI client was created with normalized URL (with /v1)
|
||||
expect(vi.mocked(OpenAI)).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
baseURL: "https://custom.openai.com/v1",
|
||||
}),
|
||||
)
|
||||
})
|
||||
|
||||
it("should preserve base URLs with /v1 suffix", () => {
|
||||
const customBaseUrl = "https://custom.openai.com/v1"
|
||||
const handlerWithCustomUrl = new OpenAiHandler({
|
||||
...mockOptions,
|
||||
openAiBaseUrl: customBaseUrl,
|
||||
})
|
||||
expect(handlerWithCustomUrl).toBeInstanceOf(OpenAiHandler)
|
||||
// Verify the OpenAI client was created with the same URL
|
||||
expect(vi.mocked(OpenAI)).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
baseURL: "https://custom.openai.com/v1",
|
||||
}),
|
||||
)
|
||||
})
|
||||
|
||||
it("should handle URLs with trailing slash and no /v1", () => {
|
||||
const customBaseUrl = "https://custom.openai.com/"
|
||||
const handlerWithCustomUrl = new OpenAiHandler({
|
||||
...mockOptions,
|
||||
openAiBaseUrl: customBaseUrl,
|
||||
})
|
||||
expect(handlerWithCustomUrl).toBeInstanceOf(OpenAiHandler)
|
||||
// Verify the OpenAI client was created with normalized URL
|
||||
expect(vi.mocked(OpenAI)).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
baseURL: "https://custom.openai.com/v1",
|
||||
}),
|
||||
)
|
||||
})
|
||||
|
||||
it("should not add /v1 to Azure endpoints", () => {
|
||||
const azureBaseUrl = "https://test.services.ai.azure.com"
|
||||
const handlerWithAzureUrl = new OpenAiHandler({
|
||||
...mockOptions,
|
||||
openAiBaseUrl: azureBaseUrl,
|
||||
})
|
||||
expect(handlerWithAzureUrl).toBeInstanceOf(OpenAiHandler)
|
||||
// Verify the OpenAI client was created without /v1 added
|
||||
expect(vi.mocked(OpenAI)).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
baseURL: "https://test.services.ai.azure.com",
|
||||
}),
|
||||
)
|
||||
})
|
||||
|
||||
it("should not add /v1 to Azure OpenAI endpoints", () => {
|
||||
const azureOpenAiBaseUrl = "https://myorg.openai.azure.com/openai"
|
||||
const handlerWithAzureOpenAi = new OpenAiHandler({
|
||||
...mockOptions,
|
||||
openAiBaseUrl: azureOpenAiBaseUrl,
|
||||
openAiUseAzure: true,
|
||||
})
|
||||
expect(handlerWithAzureOpenAi).toBeInstanceOf(OpenAiHandler)
|
||||
// Verify that the URL was not modified (no /v1 added)
|
||||
// AzureOpenAI constructor should have been called
|
||||
const mockAzure = (globalThis as any).__mockAzureOpenAI
|
||||
expect(mockAzure).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
baseURL: "https://myorg.openai.azure.com/openai",
|
||||
}),
|
||||
)
|
||||
})
|
||||
|
||||
it("should set default headers correctly", () => {
|
||||
// Check that the OpenAI constructor was called with correct parameters
|
||||
expect(vi.mocked(OpenAI)).toHaveBeenCalledWith({
|
||||
|
|
@ -1055,6 +1149,51 @@ describe("getOpenAiModels", () => {
|
|||
expect(result).toEqual(["model-1"])
|
||||
})
|
||||
|
||||
it("should normalize base URLs without /v1 suffix in getOpenAiModels", async () => {
|
||||
const mockResponse = {
|
||||
data: {
|
||||
data: [{ id: "model-1" }],
|
||||
},
|
||||
}
|
||||
vi.mocked(axios.get).mockResolvedValueOnce(mockResponse)
|
||||
|
||||
const result = await getOpenAiModels("https://api.example.com", "test-key")
|
||||
|
||||
// Should add /v1 suffix to the URL
|
||||
expect(axios.get).toHaveBeenCalledWith("https://api.example.com/v1/models", expect.any(Object))
|
||||
expect(result).toEqual(["model-1"])
|
||||
})
|
||||
|
||||
it("should handle base URLs with /v1 suffix in getOpenAiModels", async () => {
|
||||
const mockResponse = {
|
||||
data: {
|
||||
data: [{ id: "model-1" }],
|
||||
},
|
||||
}
|
||||
vi.mocked(axios.get).mockResolvedValueOnce(mockResponse)
|
||||
|
||||
const result = await getOpenAiModels("https://api.example.com/v1", "test-key")
|
||||
|
||||
// Should not add another /v1 suffix
|
||||
expect(axios.get).toHaveBeenCalledWith("https://api.example.com/v1/models", expect.any(Object))
|
||||
expect(result).toEqual(["model-1"])
|
||||
})
|
||||
|
||||
it("should handle base URLs with trailing slash in getOpenAiModels", async () => {
|
||||
const mockResponse = {
|
||||
data: {
|
||||
data: [{ id: "model-1" }],
|
||||
},
|
||||
}
|
||||
vi.mocked(axios.get).mockResolvedValueOnce(mockResponse)
|
||||
|
||||
const result = await getOpenAiModels("https://api.example.com/", "test-key")
|
||||
|
||||
// Should handle trailing slash and add /v1
|
||||
expect(axios.get).toHaveBeenCalledWith("https://api.example.com/v1/models", expect.any(Object))
|
||||
expect(result).toEqual(["model-1"])
|
||||
})
|
||||
|
||||
it("should return empty array for invalid URL after trimming", async () => {
|
||||
const result = await getOpenAiModels(" not-a-valid-url ", "test-key")
|
||||
expect(result).toEqual([])
|
||||
|
|
|
|||
|
|
@ -38,7 +38,11 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl
|
|||
super()
|
||||
this.options = options
|
||||
|
||||
const baseURL = this.options.openAiBaseUrl ?? "https://api.openai.com/v1"
|
||||
// Normalize base URL to ensure it works with or without "/v1" suffix
|
||||
const baseURL = normalizeOpenAiBaseUrl(
|
||||
this.options.openAiBaseUrl ?? "https://api.openai.com/v1",
|
||||
options.openAiUseAzure,
|
||||
)
|
||||
const apiKey = this.options.openAiApiKey ?? "not-provided"
|
||||
const isAzureAiInference = this._isAzureAiInference(this.options.openAiBaseUrl)
|
||||
const urlHost = this._getUrlHost(this.options.openAiBaseUrl)
|
||||
|
|
@ -546,16 +550,55 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Helper function to normalize OpenAI-compatible base URLs.
|
||||
* Exported for use in getOpenAiModels function.
|
||||
*/
|
||||
function normalizeOpenAiBaseUrl(baseUrl: string, isAzure: boolean = false): string {
|
||||
// Trim whitespace
|
||||
let normalizedUrl = baseUrl.trim()
|
||||
|
||||
// Get host for special case detection
|
||||
let urlHost = ""
|
||||
try {
|
||||
urlHost = new URL(normalizedUrl).host
|
||||
} catch {
|
||||
// Invalid URL, return as-is
|
||||
return normalizedUrl
|
||||
}
|
||||
|
||||
// Special case: don't modify Azure or other special endpoints
|
||||
const isAzureAiInference = urlHost.endsWith(".services.ai.azure.com")
|
||||
const isAzureOpenAi = urlHost === "azure.com" || urlHost.endsWith(".azure.com") || isAzure
|
||||
|
||||
// Azure endpoints don't use /v1 suffix
|
||||
if (isAzureAiInference || isAzureOpenAi) {
|
||||
return normalizedUrl
|
||||
}
|
||||
|
||||
// For standard OpenAI-compatible endpoints, ensure /v1 suffix
|
||||
// Remove trailing slash first
|
||||
normalizedUrl = normalizedUrl.replace(/\/$/, "")
|
||||
|
||||
// Check if it already ends with /v1 or /v1/ (case-insensitive)
|
||||
if (!/\/v1$/i.test(normalizedUrl)) {
|
||||
// Add /v1 suffix if not present
|
||||
normalizedUrl = `${normalizedUrl}/v1`
|
||||
}
|
||||
|
||||
return normalizedUrl
|
||||
}
|
||||
|
||||
export async function getOpenAiModels(baseUrl?: string, apiKey?: string, openAiHeaders?: Record<string, string>) {
|
||||
try {
|
||||
if (!baseUrl) {
|
||||
return []
|
||||
}
|
||||
|
||||
// Trim whitespace from baseUrl to handle cases where users accidentally include spaces
|
||||
const trimmedBaseUrl = baseUrl.trim()
|
||||
// Normalize the base URL
|
||||
const normalizedUrl = normalizeOpenAiBaseUrl(baseUrl, false)
|
||||
|
||||
if (!URL.canParse(trimmedBaseUrl)) {
|
||||
if (!URL.canParse(normalizedUrl)) {
|
||||
return []
|
||||
}
|
||||
|
||||
|
|
@ -573,7 +616,7 @@ export async function getOpenAiModels(baseUrl?: string, apiKey?: string, openAiH
|
|||
config["headers"] = headers
|
||||
}
|
||||
|
||||
const response = await axios.get(`${trimmedBaseUrl}/models`, config)
|
||||
const response = await axios.get(`${normalizedUrl}/models`, config)
|
||||
const modelsArray = response.data?.data?.map((model: any) => model.id) || []
|
||||
return [...new Set<string>(modelsArray)]
|
||||
} catch (error) {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue