Skip to content

Conversation

@roomote
Copy link
Contributor

@roomote roomote bot commented Sep 24, 2025

Description

This PR fixes an issue where custom OpenAI-compatible API URLs that already end with /v1 were getting another /v1 appended, resulting in /v1/v1 which caused API requests to fail.

Problem

When users configure a custom OpenAI-compatible API URL that already includes /v1 (e.g., https://custom.api.com/v1), the system was appending another /v1, creating invalid URLs like https://custom.api.com/v1/v1/chat/completions.

Solution

  • Added a normalizeBaseUrl() method to the OpenAiHandler class that:

    • Ensures /v1 is present for OpenAI-compatible APIs
    • Prevents duplication if /v1 is already present
    • Preserves Azure endpoints without modification
    • Handles URLs with trailing slashes correctly
  • Applied the same normalization logic to the getOpenAiModels() function

Testing

  • Added comprehensive test coverage for URL normalization scenarios
  • All existing tests continue to pass
  • Verified with multiple URL formats including:
    • URLs already ending with /v1
    • URLs without /v1
    • URLs with trailing slashes
    • Azure OpenAI endpoints
    • Azure AI Inference Service endpoints

Related Issue

Fixes #8282

Checklist

  • Code follows project conventions
  • Tests added and passing
  • Linting passes
  • Type checking passes
  • No breaking changes introduced

Important

Fixes URL normalization in OpenAiHandler and getOpenAiModels() to prevent /v1 duplication for OpenAI-compatible APIs, with tests added.

  • Behavior:
    • Adds normalizeBaseUrl() to OpenAiHandler to ensure /v1 is present for OpenAI-compatible APIs without duplication.
    • Updates getOpenAiModels() to apply the same URL normalization logic.
    • Preserves Azure endpoints without modification.
  • Testing:
    • Adds tests for URL normalization in openai.spec.ts covering various URL formats, including those with trailing slashes and Azure endpoints.
    • Verifies that URLs already ending with /v1 are not modified.
    • Ensures URLs without /v1 have it added correctly.

This description was created by Ellipsis for b736800. You can customize this summary. It will automatically update as commits are pushed.

- 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
@roomote roomote bot requested review from cte, jr and mrubens as code owners September 24, 2025 08:14
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Sep 24, 2025
}

/**
* Normalizes the base URL to ensure it ends with /v1 for OpenAI-compatible APIs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The URL normalization logic (adding '/v1') is correctly implemented, but note that similar logic is repeated in both the normalizeBaseUrl method (lines 427–459) and in getOpenAiModels (lines 487–509). Consider extracting this logic into a shared utility function to reduce duplication and ease future maintenance.

This comment was generated because it violated a code review rule: irule_tTqpIuNs8DV0QFGj.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Sep 24, 2025
Copy link
Contributor Author

@roomote roomote bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review: compiling my own patch like a snake eating its tail—hope the tail has tests.

Findings:
P1: Query/fragment handling in URL normalization can produce invalid URLs. Use the URL API and modify pathname only; preserve search/hash. Affects normalizeBaseUrl() and getOpenAiModels().
P2: Duplicate normalization logic between the class method and getOpenAiModels(); extract a shared helper. Also build the models URL via new URL('models', base) to avoid double slashes and preserve query.
P3: Use the normalized base for provider checks (e.g., _isGrokXAI) for consistency. Add tests covering base URLs with ?query and #fragment for both handler and model listing.

@daniel-lxs daniel-lxs closed this Sep 24, 2025
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Sep 24, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Sep 24, 2025
@daniel-lxs daniel-lxs deleted the fix/openai-custom-url-v1-duplication branch September 24, 2025 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[BUG] OpenAI接口自定义URL有点问题

4 participants