Skip to content

Conversation

@roomote
Copy link
Contributor

@roomote roomote bot commented Jul 22, 2025

This PR fixes issue #6078 where trailing slashes in Ollama API URLs were causing 405 errors and misleading error messages about models not supporting embeddings.

Problem

When users configure their Ollama base URL with a trailing slash (e.g., http://localhost:11434/), the URL concatenation in the code would create double slashes (e.g., http://localhost:11434//api/embed), leading to API call failures.

Solution

  • Created a new url-normalization utility that removes trailing slashes from base URLs
  • Added a joinUrlPath function to safely join base URLs with paths
  • Updated all Ollama-related code to use these utilities:
    • src/api/providers/ollama.ts
    • src/services/code-index/embedders/ollama.ts
    • src/api/providers/fetchers/ollama.ts

Testing

  • Added comprehensive unit tests for the URL normalization utility
  • All existing tests pass without regression
  • The fix ensures that URLs are properly normalized regardless of whether users include trailing slashes

This change improves the user experience by preventing confusing errors when configuring Ollama endpoints.


Important

Fixes issue #6078 by normalizing Ollama API URLs to handle trailing slashes, preventing 405 errors.

  • Behavior:
  • Testing:
    • Adds unit tests in url-normalization.test.ts to verify URL normalization and joining behavior.
    • Ensures all existing tests pass without regression.

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

- Added url-normalization utility to remove trailing slashes from base URLs
- Updated Ollama provider, embedder, and fetcher to use URL normalization
- This prevents double slashes in API endpoints when users include trailing slashes
- Added comprehensive tests for URL normalization utility

Fixes #6078
@roomote roomote bot requested review from cte, jr and mrubens as code owners July 22, 2025 20:53
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Jul 22, 2025
@Naam
Copy link
Contributor

Naam commented Jul 22, 2025

So that's essentially a duplicate of the one I proposed in #6079 ?

*/
export function normalizeBaseUrl(url: string): string {
// Remove all trailing slashes
return url.replace(/\/+$/, "")

Check failure

Code scanning / CodeQL

Polynomial regular expression used on uncontrolled data High

This
regular expression
that depends on
library input
may run slow on strings with many repetitions of '/'.
This
regular expression
that depends on
library input
may run slow on strings with many repetitions of '/'.
This
regular expression
that depends on
library input
may run slow on strings with many repetitions of '/'.
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jul 22, 2025
@daniel-lxs
Copy link
Member

@Naam This is an automated PR that you can ignore, I'll close it since I prefer the implementation on #6079

@daniel-lxs daniel-lxs closed this Jul 23, 2025
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Jul 23, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jul 23, 2025
@daniel-lxs daniel-lxs deleted the fix/ollama-url-normalization branch July 23, 2025 21:56
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.

5 participants