From b149b07ae190f30767e7c00a4d472d8fcabf4c7b Mon Sep 17 00:00:00 2001 From: CyberWolf Date: Thu, 5 Nov 2026 03:23:25 +0800 Subject: [PATCH 1/2] fix(provider): make the "no suffix" option take effect (#89) "" means "never configured" for new and preset providers, so it could not also mean "no suffix": the runtime fell back to the default suffix and the settings page showed the default again on reopen. Store an explicit "none" marker for "no suffix" and keep "" as "use the default", so existing providers request the same URLs as before. The meaning of the stored value now lives in src/shared/providerSuffix.ts, used by both ModelService and the settings page, so the preview matches the real URL. Gemini appends the model path itself, so the factory passes exactEndpoint to keep the request on the base URL. "No suffix" without a base URL now fails instead of falling back to the official endpoint with a gateway's key. --- src/core/application/services/ModelService.ts | 28 ++--- .../services/__tests__/ModelService.test.ts | 34 +++++- .../__tests__/ProviderEndpoint.test.ts | 82 ++++++++++++++ .../adapters/llm/GeminiClient.ts | 7 +- .../infrastructure/adapters/llm/LLMClient.ts | 11 +- .../llm/__tests__/GeminiClient.test.ts | 20 ++++ .../llm/__tests__/LLMClientFactory.test.ts | 8 ++ src/renderer/components/Provider.tsx | 71 +++--------- .../components/__tests__/Provider.test.tsx | 102 ++++++++++++++++++ src/shared/__tests__/providerSuffix.test.ts | 58 ++++++++++ src/shared/providerSuffix.ts | 31 ++++++ vitest.config.ts | 1 + 12 files changed, 369 insertions(+), 84 deletions(-) create mode 100644 src/core/application/services/__tests__/ProviderEndpoint.test.ts create mode 100644 src/shared/__tests__/providerSuffix.test.ts create mode 100644 src/shared/providerSuffix.ts diff --git a/src/core/application/services/ModelService.ts b/src/core/application/services/ModelService.ts index 75ce49d..b15f2de 100644 --- a/src/core/application/services/ModelService.ts +++ b/src/core/application/services/ModelService.ts @@ -1,5 +1,6 @@ import providerRepository from '../../domain/repositories/ProviderRepository.js'; import { LLMClientFactory, CompletionOptions, Message } from '../../infrastructure/adapters/llm/LLMClient.js'; +import { NO_SUFFIX, resolveSuffix } from '../../../shared/providerSuffix.js'; import sharp from 'sharp'; @@ -11,9 +12,8 @@ const getLLMClient = async (providerId: number) => { throw new Error('服务商不存在'); } - // 获取 base_url 和 suffix let baseUrl = provider.base_url || ''; - let suffix = provider.suffix || ''; + const isExactEndpoint = provider.suffix === NO_SUFFIX; // 根据 provider 类型设置默认值 if (!baseUrl) { @@ -24,30 +24,16 @@ const getLLMClient = async (providerId: number) => { } } - if (!suffix) { - switch (provider.type) { - case 'openai': - suffix = '/chat/completions'; - break; - case 'openai-responses': - suffix = '/responses'; - break; - case 'gemini': - suffix = '/models'; - break; - case 'anthropic': - suffix = '/messages'; - break; - case 'ollama': - suffix = '/chat'; - break; - } + // 客户端会把空地址回退到官方端点，不能把自定义网关的 Key 发过去 + if (isExactEndpoint && !baseUrl) { + throw new Error('选择「无后缀」时必须填写 API 地址'); } return LLMClientFactory.createClient( provider.type || '', provider.api_key || '', - `${baseUrl}${suffix}` + `${baseUrl}${resolveSuffix(provider.type, provider.suffix)}`, + { exactEndpoint: isExactEndpoint } ); } diff --git a/src/core/application/services/__tests__/ModelService.test.ts b/src/core/application/services/__tests__/ModelService.test.ts index 532e45a..d4506c9 100644 --- a/src/core/application/services/__tests__/ModelService.test.ts +++ b/src/core/application/services/__tests__/ModelService.test.ts @@ -72,7 +72,8 @@ describe('Model Logic', () => { expect(mockLLMClientFactory.createClient).toHaveBeenCalledWith( 'openai', 'sk-test-key', - 'https://api.openai.com/v1/chat/completions' + 'https://api.openai.com/v1/chat/completions', + { exactEndpoint: false } ) }) @@ -100,7 +101,8 @@ describe('Model Logic', () => { expect(mockLLMClientFactory.createClient).toHaveBeenCalledWith( 'ollama', '', - 'http://localhost:11434/api/chat' + 'http://localhost:11434/api/chat', + { exactEndpoint: false } ) }) @@ -137,7 +139,8 @@ describe('Model Logic', () => { expect(mockLLMClientFactory.createClient).toHaveBeenCalledWith( testCase.type, 'test-key', - `https://api.test.com${testCase.expectedSuffix}` + `https://api.test.com${testCase.expectedSuffix}`, + { exactEndpoint: false } ) } }) @@ -166,7 +169,30 @@ describe('Model Logic', () => { expect(mockLLMClientFactory.createClient).toHaveBeenCalledWith( 'openai', 'test-key', - 'https://api.custom.com/custom/endpoint' + 'https://api.custom.com/custom/endpoint', + { exactEndpoint: false } + ) + }) + + it('should pass the base URL as the exact endpoint for the no-suffix marker', async () => { + mockProviderRepository.findById.mockResolvedValue({ + id: 1, + type: 'gemini', + api_key: 'test-key', + base_url: 'https://gateway.test/full/endpoint', + suffix: 'none' + }) + mockLLMClientFactory.createClient.mockResolvedValue({ + completion: vi.fn().mockResolvedValue({ content: 'response' }) + }) + + await modelLogic.completion(1, { messages: [], model: 'test' }) + + expect(mockLLMClientFactory.createClient).toHaveBeenCalledWith( + 'gemini', + 'test-key', + 'https://gateway.test/full/endpoint', + { exactEndpoint: true } ) }) }) diff --git a/src/core/application/services/__tests__/ProviderEndpoint.test.ts b/src/core/application/services/__tests__/ProviderEndpoint.test.ts new file mode 100644 index 0000000..d8f09b2 --- /dev/null +++ b/src/core/application/services/__tests__/ProviderEndpoint.test.ts @@ -0,0 +1,82 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { NO_SUFFIX } from '../../../../shared/providerSuffix.js' + +/** + * The URL each provider type really requests, for each stored suffix. + * + * Only the database, sharp and fetch are mocked: ModelService builds the URL and + * the real clients may extend it, so testing either side alone cannot catch a + * suffix that does not reach the wire (issue #89). + */ + +const mockProviderRepository = { findById: vi.fn() } + +vi.mock('../../../domain/repositories/ProviderRepository.js', () => ({ + default: mockProviderRepository +})) + +vi.mock('sharp', () => ({ + default: vi.fn(() => ({ toBuffer: vi.fn().mockResolvedValue(Buffer.from('page')) })) +})) + +const BASE = 'https://proxy.test/v1/my-endpoint' +const MODEL = 'some-model' + +// A body every client can parse, each reads only the fields of its own protocol +const ANY_PROTOCOL_BODY = { + choices: [{ message: { content: 'ok' }, finish_reason: 'stop' }], + output: [{ content: 'ok' }], + content: [{ type: 'text', text: 'ok' }], + candidates: [{ content: { parts: [{ text: 'ok' }] }, finishReason: 'STOP' }] +} + +const CASES = [ + { type: 'openai', defaultUrl: `${BASE}/chat/completions` }, + { type: 'openai-responses', defaultUrl: `${BASE}/responses` }, + { type: 'anthropic', defaultUrl: `${BASE}/messages` }, + { type: 'gemini', defaultUrl: `${BASE}/models/${MODEL}:generateContent` } +] + +describe('Provider request URL', () => { + let modelService: any + const fetchMock = vi.fn() + + const requestedUrl = async (type: string, suffix: string | null, base_url = BASE) => { + mockProviderRepository.findById.mockResolvedValue({ id: 1, type, api_key: 'key', base_url, suffix }) + const result = await modelService.completion(1, { + model: MODEL, + messages: [{ role: 'user', content: { type: 'text', text: 'hi' } }] + }) + expect(result.content).toBe('ok') + return String(fetchMock.mock.calls[0][0]) + } + + beforeEach(async () => { + fetchMock.mockReset() + fetchMock.mockImplementation(async () => new Response(JSON.stringify(ANY_PROTOCOL_BODY), { status: 200 })) + global.fetch = fetchMock as unknown as typeof fetch + vi.spyOn(console, 'log').mockImplementation(() => {}) + modelService = (await import('../ModelService.js')).default + }) + + describe.each(CASES)('$type', ({ type, defaultUrl }) => { + it('requests exactly the base URL when "no suffix" is selected', async () => { + expect(await requestedUrl(type, NO_SUFFIX)).toBe(BASE) + }) + + it.each([[''], [null]])('keeps the default URL of an existing provider with suffix %j', async (suffix) => { + expect(await requestedUrl(type, suffix)).toBe(defaultUrl) + }) + + it('rejects "no suffix" without a base URL instead of calling the official endpoint', async () => { + mockProviderRepository.findById.mockResolvedValue({ id: 1, type, api_key: 'key', base_url: '', suffix: NO_SUFFIX }) + + await expect(modelService.completion(1, { model: MODEL, messages: [] })).rejects.toThrow('无后缀') + expect(fetchMock).not.toHaveBeenCalled() + }) + }) + + it('still adds the model path to a custom Gemini suffix', async () => { + expect(await requestedUrl('gemini', '/custom')).toBe(`${BASE}/custom/${MODEL}:generateContent`) + }) +}) diff --git a/src/core/infrastructure/adapters/llm/GeminiClient.ts b/src/core/infrastructure/adapters/llm/GeminiClient.ts index adfa59e..32da8f1 100644 --- a/src/core/infrastructure/adapters/llm/GeminiClient.ts +++ b/src/core/infrastructure/adapters/llm/GeminiClient.ts @@ -12,7 +12,10 @@ import { * Google Gemini客户端实现 */ export class GeminiClient extends LLMClient { - constructor(apiKey: string, baseUrl?: string) { + /** + * @param exactEndpoint baseUrl 已是完整地址（如网关端点），不再追加模型路径 + */ + constructor(apiKey: string, baseUrl?: string, private readonly exactEndpoint: boolean = false) { super(apiKey, baseUrl || 'https://generativelanguage.googleapis.com/v1beta/models'); } @@ -26,7 +29,7 @@ export class GeminiClient extends LLMClient { const modelName = normalizedOptions.model || 'gemini-1.5-pro'; // 使用 v1beta 端点，key 通过 header 传递 - const endpoint = `${this.baseUrl}/${modelName}:generateContent`; + const endpoint = this.exactEndpoint ? this.baseUrl : `${this.baseUrl}/${modelName}:generateContent`; // 将消息转换为Gemini格式 const geminiContents = this.convertMessagesToGeminiFormat(normalizedOptions.messages); diff --git a/src/core/infrastructure/adapters/llm/LLMClient.ts b/src/core/infrastructure/adapters/llm/LLMClient.ts index f30a7b9..765ebca 100644 --- a/src/core/infrastructure/adapters/llm/LLMClient.ts +++ b/src/core/infrastructure/adapters/llm/LLMClient.ts @@ -97,8 +97,15 @@ export class LLMClientFactory { * @param type LLM client type * @param apiKey API key * @param baseUrl Base URL (some services need custom URL) + * @param options.exactEndpoint baseUrl is the complete endpoint, the client must not extend it. + * Only Gemini extends its URL (with the model path); the other clients always post to baseUrl. */ - static async createClient(type: string, apiKey: string, baseUrl?: string): Promise { + static async createClient( + type: string, + apiKey: string, + baseUrl?: string, + options: { exactEndpoint?: boolean } = {} + ): Promise { switch (type) { case 'openai': { const OpenAIModule = await import('./OpenAIClient.js'); @@ -110,7 +117,7 @@ export class LLMClientFactory { } case 'gemini': { const GeminiModule = await import('./GeminiClient.js'); - return new GeminiModule.GeminiClient(apiKey, baseUrl || ''); + return new GeminiModule.GeminiClient(apiKey, baseUrl || '', options.exactEndpoint); } case 'anthropic': { const AnthropicModule = await import('./AnthropicClient.js'); diff --git a/src/core/infrastructure/adapters/llm/__tests__/GeminiClient.test.ts b/src/core/infrastructure/adapters/llm/__tests__/GeminiClient.test.ts index 94bb307..5343498 100644 --- a/src/core/infrastructure/adapters/llm/__tests__/GeminiClient.test.ts +++ b/src/core/infrastructure/adapters/llm/__tests__/GeminiClient.test.ts @@ -22,6 +22,26 @@ describe('GeminiClient', () => { }) }) + describe('Endpoint', () => { + const request = { messages: [{ role: 'user' as const, content: { type: 'text' as const, text: 'Hi' } }], model: 'gemini-pro' } + + it('should add the model path to the base URL by default', async () => { + mockFetchSuccess(createMockGeminiResponse('ok')) + + await new GeminiClient('key', 'https://gateway.test/models').completion(request) + + expect((global.fetch as any).mock.calls[0][0]).toBe('https://gateway.test/models/gemini-pro:generateContent') + }) + + it('should request the base URL unchanged for an exact endpoint', async () => { + mockFetchSuccess(createMockGeminiResponse('ok')) + + await new GeminiClient('key', 'https://gateway.test/full/endpoint', true).completion(request) + + expect((global.fetch as any).mock.calls[0][0]).toBe('https://gateway.test/full/endpoint') + }) + }) + describe('Message Format Conversion', () => { it('should convert simple text message to Gemini format', async () => { const mockResponse = createMockGeminiResponse('Test response') diff --git a/src/core/infrastructure/adapters/llm/__tests__/LLMClientFactory.test.ts b/src/core/infrastructure/adapters/llm/__tests__/LLMClientFactory.test.ts index 9b9f40a..9c2857e 100644 --- a/src/core/infrastructure/adapters/llm/__tests__/LLMClientFactory.test.ts +++ b/src/core/infrastructure/adapters/llm/__tests__/LLMClientFactory.test.ts @@ -27,6 +27,14 @@ describe('LLMClientFactory', () => { expect(client.constructor.name).toBe('GeminiClient') }) + it('should pass exactEndpoint to the Gemini client', async () => { + const exact = await LLMClientFactory.createClient('gemini', 'key', 'https://gateway.test', { exactEndpoint: true }) + const regular = await LLMClientFactory.createClient('gemini', 'key', 'https://gateway.test') + + expect(exact['exactEndpoint' as keyof typeof exact]).toBe(true) + expect(regular['exactEndpoint' as keyof typeof regular]).toBe(false) + }) + it('should create Ollama client', async () => { const client = await LLMClientFactory.createClient('ollama', '', 'http://localhost:11434') expect(client).toBeDefined() diff --git a/src/renderer/components/Provider.tsx b/src/renderer/components/Provider.tsx index 29e5714..6ff2128 100644 --- a/src/renderer/components/Provider.tsx +++ b/src/renderer/components/Provider.tsx @@ -19,6 +19,12 @@ import { } from "antd"; import React, { useCallback, useEffect, useState } from "react"; import { useTranslation } from "react-i18next"; +import { + NO_SUFFIX, + getDefaultSuffix, + getSuffixSelection, + resolveSuffix, +} from "../../shared/providerSuffix"; interface ProviderProps { providerId?: number; @@ -94,30 +100,7 @@ const Provider: React.FC = ({ } } - // 根据 provider 类型设置默认后缀 - if (!data.suffix) { - switch (data.type) { - case "openai": - setSuffix("/chat/completions"); - break; - case "openai-responses": - setSuffix("/responses"); - break; - case "gemini": - setSuffix("/models"); - break; - case "anthropic": - setSuffix("/messages"); - break; - case "ollama": - setSuffix("/chat"); - break; - default: - setSuffix(""); - } - } else { - setSuffix(data.suffix); - } + setSuffix(getSuffixSelection(data.type, data.suffix)); } catch (error) { console.error("Failed to fetch provider details:", error); } @@ -420,7 +403,7 @@ const Provider: React.FC = ({ {t('details.api_url_label')} {baseUrl} - {suffix} + {resolveSuffix(providerData?.type, suffix)} = ({ }} options={(() => { const type = providerData?.type; - const noSuffix = { label: t('details.no_suffix'), value: "" }; - switch (type) { - case "openai": - return [ - { label: "/chat/completions", value: "/chat/completions" }, - noSuffix, - ]; - case "openai-responses": - return [ - { label: "/responses", value: "/responses" }, - noSuffix, - ]; - case "gemini": - return [ - { label: "/models", value: "/models" }, - noSuffix, - ]; - case "anthropic": - return [ - { label: "/messages", value: "/messages" }, - noSuffix, - ]; - case "ollama": - // 空后缀运行时会回退到 /chat，所以不提供“无后缀” - // base 约定以 /api 结尾（模型列表依赖 base + /tags），所以不提供 /api/chat - return [ - { label: "/chat", value: "/chat" }, - ]; - default: - return [noSuffix]; + const noSuffix = { label: t('details.no_suffix'), value: NO_SUFFIX }; + const defaultSuffix = getDefaultSuffix(type); + if (!defaultSuffix) { + return [noSuffix]; } + const defaultOption = { label: defaultSuffix, value: defaultSuffix }; + // Ollama 的 base 约定以 /api 结尾（模型列表依赖 base + /tags）， + // 所以不提供“无后缀”和 /api/chat + return type === "ollama" ? [defaultOption] : [defaultOption, noSuffix]; })()} /> } diff --git a/src/renderer/components/__tests__/Provider.test.tsx b/src/renderer/components/__tests__/Provider.test.tsx index 36c56a4..4ed7d9b 100644 --- a/src/renderer/components/__tests__/Provider.test.tsx +++ b/src/renderer/components/__tests__/Provider.test.tsx @@ -3,6 +3,7 @@ import { render, screen, cleanup, waitFor, fireEvent } from '@testing-library/re import { BrowserRouter } from 'react-router-dom' import { App } from 'antd' import Provider from '../Provider' +import { NO_SUFFIX } from '../../../shared/providerSuffix' // Mock react-i18next vi.mock('react-i18next', () => ({ @@ -531,6 +532,107 @@ describe('Provider', () => { expect(suffixSelect).toBeInTheDocument() }) + describe('no suffix', () => { + const BASE = 'https://proxy.test/v1/my-endpoint' + + const TYPES = [ + { type: 'openai', defaultSuffix: '/chat/completions' }, + { type: 'openai-responses', defaultSuffix: '/responses' }, + { type: 'gemini', defaultSuffix: '/models' }, + { type: 'anthropic', defaultSuffix: '/messages' } + ] + + const renderProvider = async () => { + render( + + + + ) + + await waitFor(() => { + expect(screen.getByDisplayValue(BASE)).toBeInTheDocument() + }) + } + + // Saved values come back from getById, as they do from the database + const renderStoredProvider = async (type: string, suffix: string | null) => { + let stored = { ...mockProvider, type, base_url: BASE, suffix } + vi.mocked(window.api.provider.getById).mockImplementation(async () => ({ success: true, data: stored })) + vi.mocked(window.api.provider.update).mockImplementation(async (_id, data) => { + stored = { ...stored, ...data } + return { success: true, data: stored } + }) + + await renderProvider() + } + + const selectedSuffix = () => + Array.from(document.querySelectorAll('.ant-select-selection-item')).map(el => el.textContent) + + // The preview renders base URL and suffix as separate text nodes + const previewText = () => screen.getByText('API URL').nextElementSibling?.textContent + + describe.each(TYPES)('$type', ({ type, defaultSuffix }) => { + it('should restore No Suffix on reopen and preview the exact base URL', async () => { + await renderStoredProvider(type, NO_SUFFIX) + + await waitFor(() => { + expect(selectedSuffix()).toEqual(['No Suffix']) + }) + expect(previewText()).toBe(BASE) + }) + + it.each([[''], [null]])('should show the default for an existing provider with suffix %j', async (suffix) => { + await renderStoredProvider(type, suffix) + + await waitFor(() => { + expect(selectedSuffix()).toEqual([defaultSuffix]) + }) + expect(previewText()).toBe(`${BASE}${defaultSuffix}`) + }) + + it('should save No Suffix, update the preview and still show it after reopening', async () => { + await renderStoredProvider(type, '') + + await waitFor(() => { + expect(selectedSuffix()).toEqual([defaultSuffix]) + }) + fireEvent.mouseDown(document.querySelector('.ant-select-selector')!) + fireEvent.click(await screen.findByText('No Suffix')) + + await waitFor(() => { + expect(window.api.provider.update).toHaveBeenCalledWith(1, { suffix: NO_SUFFIX }) + }) + await waitFor(() => { + expect(previewText()).toBe(BASE) + }) + + cleanup() + await renderProvider() + + await waitFor(() => { + expect(selectedSuffix()).toEqual(['No Suffix']) + }) + expect(previewText()).toBe(BASE) + }) + }) + + it('should keep No Suffix when the base URL is edited', async () => { + await renderStoredProvider('openai', NO_SUFFIX) + + const input = screen.getByDisplayValue(BASE) + fireEvent.change(input, { target: { value: 'https://proxy.test/other' } }) + fireEvent.blur(input) + + await waitFor(() => { + expect(window.api.provider.update).toHaveBeenCalledWith(1, { + base_url: 'https://proxy.test/other', + suffix: NO_SUFFIX + }) + }) + }) + }) + describe('ollama', () => { const mockOllama = (suffix: string) => { vi.mocked(window.api.provider.getById).mockResolvedValue({ diff --git a/src/shared/__tests__/providerSuffix.test.ts b/src/shared/__tests__/providerSuffix.test.ts new file mode 100644 index 0000000..21e0749 --- /dev/null +++ b/src/shared/__tests__/providerSuffix.test.ts @@ -0,0 +1,58 @@ +import { describe, it, expect } from 'vitest' +import { + NO_SUFFIX, + getDefaultSuffix, + getSuffixSelection, + resolveSuffix +} from '../providerSuffix.js' + +const TYPES_WITH_DEFAULT = [ + ['openai', '/chat/completions'], + ['openai-responses', '/responses'], + ['gemini', '/models'], + ['anthropic', '/messages'], + ['ollama', '/chat'] +] as const + +describe('providerSuffix', () => { + describe.each(TYPES_WITH_DEFAULT)('%s', (type, defaultSuffix) => { + it('falls back to the default for an unset suffix', () => { + for (const unset of ['', null, undefined]) { + expect(getSuffixSelection(type, unset)).toBe(defaultSuffix) + expect(resolveSuffix(type, unset)).toBe(defaultSuffix) + } + }) + + it('appends nothing for the no-suffix marker but keeps it selected', () => { + expect(getSuffixSelection(type, NO_SUFFIX)).toBe(NO_SUFFIX) + expect(resolveSuffix(type, NO_SUFFIX)).toBe('') + }) + + it('keeps an explicit suffix', () => { + expect(getSuffixSelection(type, '/custom')).toBe('/custom') + expect(resolveSuffix(type, '/custom')).toBe('/custom') + }) + }) + + it('never treats the marker as a path', () => { + expect(NO_SUFFIX).not.toBe('') + expect(NO_SUFFIX.startsWith('/')).toBe(false) + }) + + it('selects no suffix for a type without a default, whatever the unset value', () => { + expect(getDefaultSuffix('unknown')).toBe('') + expect(getDefaultSuffix(null)).toBe('') + expect(getSuffixSelection('unknown', '')).toBe(NO_SUFFIX) + expect(resolveSuffix('unknown', '')).toBe('') + }) + + it('is stable when the selection is resolved again', () => { + for (const [type] of TYPES_WITH_DEFAULT) { + for (const stored of ['', NO_SUFFIX, '/custom']) { + const selection = getSuffixSelection(type, stored) + expect(getSuffixSelection(type, selection)).toBe(selection) + expect(resolveSuffix(type, selection)).toBe(resolveSuffix(type, stored)) + } + } + }) +}) diff --git a/src/shared/providerSuffix.ts b/src/shared/providerSuffix.ts new file mode 100644 index 0000000..6b303b7 --- /dev/null +++ b/src/shared/providerSuffix.ts @@ -0,0 +1,31 @@ +/** + * Meaning of the value stored in `Provider.suffix`: + * - `""` / `null`: never configured, the default suffix of the provider type applies. + * New and preset providers are stored this way, so it cannot also mean "no suffix". + * - `NO_SUFFIX`: the user chose "no suffix", the base URL is the complete endpoint. + * - anything else: the path appended to the base URL. + */ +export const NO_SUFFIX = 'none'; + +const DEFAULT_SUFFIXES: Record = { + openai: '/chat/completions', + 'openai-responses': '/responses', + gemini: '/models', + anthropic: '/messages', + ollama: '/chat', +}; + +type Nullable = T | null | undefined; + +export const getDefaultSuffix = (type: Nullable): string => + DEFAULT_SUFFIXES[type ?? ''] ?? ''; + +/** The value the suffix dropdown shows. A type without a default suffix has nothing else to show. */ +export const getSuffixSelection = (type: Nullable, suffix: Nullable): string => + suffix || getDefaultSuffix(type) || NO_SUFFIX; + +/** The path actually appended to the base URL. */ +export const resolveSuffix = (type: Nullable, suffix: Nullable): string => { + const selection = getSuffixSelection(type, suffix); + return selection === NO_SUFFIX ? '' : selection; +}; diff --git a/vitest.config.ts b/vitest.config.ts index fc2085c..a0b362a 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -9,6 +9,7 @@ export default defineConfig({ "src/main/**/*.test.ts", "src/core/**/*.test.ts", "src/preload/**/*.test.ts", + "src/shared/**/*.test.ts", "scripts/**/*.test.mjs", ], exclude: ["**/node_modules/**", "**/dist/**", "**/*.d.ts"], From 51dfb407c89f89d59f41bb549cc1d4882ed11a60 Mon Sep 17 00:00:00 2001 From: CyberWolf Date: Thu, 5 Nov 2026 03:23:25 +0800 Subject: [PATCH 2/2] fix(ollama): send Bearer token on completion requests (#89) The model list sent the API key but completions did not, so an Ollama behind an authenticating proxy listed models and then failed with HTTP 401 on connection test and conversion. Send the header only when a key is set. A custom-named Ollama provider matched no preset, so its model list went to /models, which Ollama does not serve. Fall back to the Ollama preset (/tags and the default base URL). --- .../services/__tests__/OllamaProtocol.test.ts | 49 ++++++++++++++++++- .../adapters/llm/OllamaClient.ts | 4 ++ .../llm/__tests__/OllamaClient.test.ts | 29 +++++++++++ .../__tests__/provider.handler.test.ts | 41 ++++++++++++++++ src/main/ipc/handlers/provider.handler.ts | 5 +- 5 files changed, 125 insertions(+), 3 deletions(-) diff --git a/src/core/application/services/__tests__/OllamaProtocol.test.ts b/src/core/application/services/__tests__/OllamaProtocol.test.ts index e324d38..e241b7a 100644 --- a/src/core/application/services/__tests__/OllamaProtocol.test.ts +++ b/src/core/application/services/__tests__/OllamaProtocol.test.ts @@ -73,11 +73,11 @@ const fakeOllamaServer = vi.fn(async (input: string | URL, init?: RequestInit): describe('Ollama protocol consistency', () => { let modelService: any - const useProvider = (base_url: string, suffix: string | null) => { + const useProvider = (base_url: string, suffix: string | null, api_key = '') => { mockProviderRepository.findById.mockResolvedValue({ id: 1, type: 'ollama', - api_key: '', + api_key, base_url, suffix }) @@ -158,6 +158,51 @@ describe('Ollama protocol consistency', () => { }) }) + describe('reverse proxy with Bearer authentication (issue #89)', () => { + const API_KEY = 'proxy-key' + + const testConnection = () => + modelService.completion(1, { + model: 'llama3.2-vision', + maxTokens: 8, + messages: [{ + role: 'user', + content: [ + { type: 'image_url', image_url: { url: `data:image/png;base64,${PAGE_IMAGE_BASE64}` } }, + { type: 'text', text: 'Identify the largest letter.' } + ] + }] + }) + + beforeEach(() => { + global.fetch = vi.fn(async (input: string | URL, init?: RequestInit) => + new Headers(init?.headers).get('Authorization') === `Bearer ${API_KEY}` + ? fakeOllamaServer(input, init) + : new Response('unauthorized', { status: 401, statusText: 'Unauthorized' }) + ) as unknown as typeof fetch + }) + + it.each([ + ['page conversion', convertPage], + ['connection test', testConnection] + ])('authenticates the %s', async (_name, request) => { + useProvider(`${HOST}/api`, '', API_KEY) + + const result = await request() + + expect(result.content).toBe(MARKDOWN) + }) + + it('sends no Authorization header when no key is configured', async () => { + useProvider(`${HOST}/api`, '') + + await expect(convertPage()).rejects.toThrow(/HTTP 401/) + + const init = vi.mocked(global.fetch).mock.calls[0][1] + expect(new Headers(init?.headers).has('Authorization')).toBe(false) + }) + }) + describe('misconfigured endpoints', () => { it('reports the endpoint instead of returning empty content for a non-chat response', async () => { useProvider(`${HOST}/api`, '/generate') diff --git a/src/core/infrastructure/adapters/llm/OllamaClient.ts b/src/core/infrastructure/adapters/llm/OllamaClient.ts index 29408f9..073076e 100644 --- a/src/core/infrastructure/adapters/llm/OllamaClient.ts +++ b/src/core/infrastructure/adapters/llm/OllamaClient.ts @@ -60,10 +60,14 @@ export class OllamaClient extends LLMClient { requestBody.format = 'json'; } + // 本地 Ollama 无需鉴权，只有配置了 Key（如带鉴权的反向代理）才发送 + const apiKey = normalizedOptions.apiKey || this.apiKey; + const response = await fetch(`${this.baseUrl}`, { method: 'POST', headers: { 'Content-Type': 'application/json', + ...(apiKey ? { 'Authorization': `Bearer ${apiKey}` } : {}), 'X-Title': 'MarkPDFdown', 'HTTP-Referer': 'https://github.com/MarkPDFdown' }, diff --git a/src/core/infrastructure/adapters/llm/__tests__/OllamaClient.test.ts b/src/core/infrastructure/adapters/llm/__tests__/OllamaClient.test.ts index fec5337..8ba581d 100644 --- a/src/core/infrastructure/adapters/llm/__tests__/OllamaClient.test.ts +++ b/src/core/infrastructure/adapters/llm/__tests__/OllamaClient.test.ts @@ -9,6 +9,35 @@ describe('OllamaClient', () => { client = new OllamaClient('', 'http://localhost:11434/api') }) + describe('Authorization', () => { + const request = { messages: [{ role: 'user' as const, content: { type: 'text' as const, text: 'Hi' } }], model: 'llama2' } + const sentHeaders = () => (global.fetch as any).mock.calls[0][1].headers + + it('should send a Bearer token when an API key is configured', async () => { + mockFetchSuccess(createMockOllamaResponse('ok')) + + await new OllamaClient('proxy-key', 'http://proxy.test/api/chat').completion(request) + + expect(sentHeaders().Authorization).toBe('Bearer proxy-key') + }) + + it('should prefer the API key of the request', async () => { + mockFetchSuccess(createMockOllamaResponse('ok')) + + await new OllamaClient('proxy-key', 'http://proxy.test/api/chat').completion({ ...request, apiKey: 'request-key' }) + + expect(sentHeaders().Authorization).toBe('Bearer request-key') + }) + + it('should not send the header without an API key', async () => { + mockFetchSuccess(createMockOllamaResponse('ok')) + + await client.completion(request) + + expect(sentHeaders()).not.toHaveProperty('Authorization') + }) + }) + describe('Constructor', () => { it('should initialize with API key and base URL', () => { expect(client).toBeDefined() diff --git a/src/main/ipc/handlers/__tests__/provider.handler.test.ts b/src/main/ipc/handlers/__tests__/provider.handler.test.ts index d6bff17..1bf4997 100644 --- a/src/main/ipc/handlers/__tests__/provider.handler.test.ts +++ b/src/main/ipc/handlers/__tests__/provider.handler.test.ts @@ -75,6 +75,7 @@ vi.mock('../../../../core/infrastructure/config/providerPresets.js', () => ({ { name: 'OpenAI', type: 'openai-responses', apiBase: 'https://api.openai.com/v1', modelListApi: '/models', modelNameField: 'id', modelIdField: 'id' }, { name: 'Anthropic', type: 'anthropic', apiBase: 'https://api.anthropic.com/v1', modelListApi: '/models', modelNameField: 'display_name', modelIdField: 'id', capabilityField: 'input_modalities', capabilityFilter: 'image' }, { name: 'OpenRouter', type: 'openai', apiBase: 'https://openrouter.ai/api/v1', modelListApi: '/models', modelNameField: 'name', modelIdField: 'id', capabilityField: 'architecture.input_modalities', capabilityFilter: 'image' }, + { name: 'Ollama', type: 'ollama', apiBase: 'http://localhost:11434/api', modelListApi: '/tags', modelNameField: 'name', modelIdField: 'name' }, ] return presets.find(p => p.type === type && p.name === name) }, @@ -599,6 +600,46 @@ describe('Provider Handler', () => { expect(result.data).toHaveLength(2) }) + describe('ollama', () => { + const listModels = async (provider: { name: string; api_key: string; base_url: string }) => { + mockProviderRepository.findById.mockResolvedValue({ id: 1, type: 'ollama', ...provider }) + vi.mocked(global.fetch).mockResolvedValue({ + ok: true, + json: vi.fn().mockResolvedValue({ models: [{ name: 'llava:latest' }] }), + text: vi.fn() + } as any) + + const handler = handlers.get('provider:fetchModelList') + const result = await handler!({}, 1) + const [url, init] = vi.mocked(global.fetch).mock.calls[0] + return { result, url, headers: init?.headers as Record } + } + + it.each(['Ollama', 'My Proxy'])('should list models of "%s" via /tags with a Bearer token', async (name) => { + const { result, url, headers } = await listModels({ + name, + api_key: 'proxy-key', + base_url: 'https://proxy.test/api' + }) + + expect(url).toBe('https://proxy.test/api/tags') + expect(headers.Authorization).toBe('Bearer proxy-key') + expect(result.data).toEqual([{ id: 'llava:latest', name: 'llava:latest' }]) + }) + + it('should not send a Bearer token without an API key', async () => { + const { headers } = await listModels({ name: 'My Proxy', api_key: '', base_url: 'https://proxy.test/api' }) + + expect(headers).not.toHaveProperty('Authorization') + }) + + it('should use the default base URL for a custom-named provider without one', async () => { + const { url } = await listModels({ name: 'My Proxy', api_key: '', base_url: '' }) + + expect(url).toBe('http://localhost:11434/api/tags') + }) + }) + it('should not filter for custom providers without preset', async () => { mockProviderRepository.findById.mockResolvedValue({ id: 1, name: 'MyCustom', type: 'openai', api_key: 'key', base_url: 'https://my-api.com/v1' diff --git a/src/main/ipc/handlers/provider.handler.ts b/src/main/ipc/handlers/provider.handler.ts index 8e427c0..89f1393 100644 --- a/src/main/ipc/handlers/provider.handler.ts +++ b/src/main/ipc/handlers/provider.handler.ts @@ -175,7 +175,10 @@ export function registerProviderHandlers() { } // Find preset configuration for this provider (may be null for custom providers) - const preset = findProviderPreset(provider.type, provider.name); + // Every Ollama server has the same API, so a custom-named one still lists models via /tags + const preset = + findProviderPreset(provider.type, provider.name) ?? + (provider.type === "ollama" ? findProviderPreset("ollama", "Ollama") : undefined); // Construct the full URL for fetching models const baseUrl = provider.base_url || preset?.apiBase;