Skip to content

feat: add Ollama API key support for Turbo mode #7425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 30, 2025
Merged

feat: add Ollama API key support for Turbo mode #7425

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2b8cdb7
feat: add Ollama API key support for Turbo mode
roomote Aug 26, 2025
465dd2f
fix: use VSCodeTextField for Ollama API key field
roomote Aug 26, 2025
0db0df5
Add missing translation keys for Ollama API key support
daniel-lxs Aug 30, 2025
00628f8
refactor: improve type safety for Ollama client configuration
daniel-lxs Aug 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/types/src/global-settings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ export const SECRET_STATE_KEYS = [
"awsSecretKey",
"awsSessionToken",
"openAiApiKey",
"ollamaApiKey",
"geminiApiKey",
"openAiNativeApiKey",
"cerebrasApiKey",
Expand Down
1 change: 1 addition & 0 deletions packages/types/src/provider-settings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ const openAiSchema = baseProviderSettingsSchema.extend({
const ollamaSchema = baseProviderSettingsSchema.extend({
ollamaModelId: z.string().optional(),
ollamaBaseUrl: z.string().optional(),
ollamaApiKey: z.string().optional(),
})

const vsCodeLmSchema = baseProviderSettingsSchema.extend({
Expand Down
11 changes: 11 additions & 0 deletions src/api/providers/__tests__/ollama.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,17 @@ describe("OllamaHandler", () => {
})
expect(handlerWithoutUrl).toBeInstanceOf(OllamaHandler)
})

it("should use API key when provided", () => {
const handlerWithApiKey = new OllamaHandler({
apiModelId: "llama2",
ollamaModelId: "llama2",
ollamaBaseUrl: "https://ollama.com",
ollamaApiKey: "test-api-key",
})
expect(handlerWithApiKey).toBeInstanceOf(OllamaHandler)
Copy link
Contributor Author

@roomote roomote bot Aug 26, 2025

Choose a reason for hiding this comment

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

The test verifies that the handler can be instantiated with an API key, but doesn't actually test that the API key is used in the Authorization header. Could we add an assertion to verify the header is properly set?

Consider mocking the OpenAI constructor to capture and verify the headers:

// The API key will be used in the Authorization header
})
})

describe("createMessage", () => {
Expand Down
13 changes: 11 additions & 2 deletions src/api/providers/native-ollama.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,19 @@ export class NativeOllamaHandler extends BaseProvider implements SingleCompletio
private ensureClient(): Ollama {
if (!this.client) {
try {
this.client = new Ollama({
const clientOptions: any = {
Copy link
Contributor Author

@roomote roomote bot Aug 26, 2025

Choose a reason for hiding this comment

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

Using type here bypasses TypeScript's type checking. Is this intentional? The Ollama client likely has a proper options interface we could use instead:

This would give us proper type safety while still allowing dynamic property assignment.

Copy link
Member

@daniel-lxs daniel-lxs Aug 30, 2025

Choose a reason for hiding this comment

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

solved

host: this.options.ollamaBaseUrl || "http://localhost:11434",
// Note: The ollama npm package handles timeouts internally
})
}

// Add API key if provided (for Ollama cloud or authenticated instances)
if (this.options.ollamaApiKey) {
clientOptions.headers = {
Authorization: `Bearer ${this.options.ollamaApiKey}`,
}
}

this.client = new Ollama(clientOptions)
} catch (error: any) {
throw new Error(`Error creating Ollama client: ${error.message}`)
}
Expand Down
12 changes: 11 additions & 1 deletion src/api/providers/ollama.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,20 @@ export class OllamaHandler extends BaseProvider implements SingleCompletionHandl
super()
this.options = options

// Use the API key if provided (for Ollama cloud or authenticated instances)
// Otherwise use "ollama" as a placeholder for local instances
const apiKey = this.options.ollamaApiKey || "ollama"

const headers: Record<string, string> = {}
if (this.options.ollamaApiKey) {
headers["Authorization"] = `Bearer ${this.options.ollamaApiKey}`
Copy link
Contributor Author

@roomote roomote bot Aug 26, 2025

Choose a reason for hiding this comment

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

Is passing the API key in both and necessary? The OpenAI client typically handles authentication via the parameter. The Authorization header in might be redundant unless Ollama's OpenAI-compatible endpoint specifically requires it.

Could we test if just using without the works?

}

this.client = new OpenAI({
baseURL: (this.options.ollamaBaseUrl || "http://localhost:11434") + "/v1",
apiKey: "ollama",
apiKey: apiKey,
timeout: getApiRequestTimeout(),
defaultHeaders: headers,
})
}

Expand Down
13 changes: 13 additions & 0 deletions webview-ui/src/components/settings/providers/Ollama.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,19 @@ export const Ollama = ({ apiConfiguration, setApiConfigurationField }: OllamaPro
className="w-full">
<label className="block font-medium mb-1">{t("settings:providers.ollama.baseUrl")}</label>
</VSCodeTextField>
{apiConfiguration?.ollamaBaseUrl && (
Copy link
Contributor Author

@roomote roomote bot Aug 26, 2025

Choose a reason for hiding this comment

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

The conditional rendering logic here is clever - only showing the API key field when a custom base URL is provided. Consider adding a comment explaining this UX decision to help future maintainers:

<VSCodeTextField
value={apiConfiguration?.ollamaApiKey || ""}
type="password"
onInput={handleInputChange("ollamaApiKey")}
placeholder={t("settings:placeholders.apiKey")}
className="w-full">
<label className="block font-medium mb-1">{t("settings:providers.ollama.apiKey")}</label>
Copy link
Contributor Author

@roomote roomote bot Aug 26, 2025

Choose a reason for hiding this comment

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

Critical issue: The translation keys and referenced here don't exist in the translation files. This will cause the UI to display missing translation keys instead of proper text.

You need to add these keys to and all other locale files:

<div className="text-xs text-vscode-descriptionForeground mt-1">
{t("settings:providers.ollama.apiKeyHelp")}
</div>
</VSCodeTextField>
)}
<VSCodeTextField
value={apiConfiguration?.ollamaModelId || ""}
onInput={handleInputChange("ollamaModelId")}
Expand Down
Loading