diff --git a/docs/AI.md b/docs/AI.md index f44281a27..7c2e626d4 100644 --- a/docs/AI.md +++ b/docs/AI.md @@ -381,7 +381,9 @@ Configure providers in the UI: **Settings → Pulse Intelligence → Provider & ChatGPT; no OpenAI API key is required or forwarded - **Claude subscription (local)** — uses an installed Claude CLI signed in with a Claude plan; no Anthropic API key is required or forwarded -- **OpenAI-compatible base URL** (for providers that implement the OpenAI API shape) +- **OpenAI-compatible base URL** (llama.cpp, LocalAI, LM Studio, and other + compatible servers; the API key is optional when the custom endpoint is + intentionally keyless) Legacy Anthropic OAuth fields may still appear in stored settings so existing installs can disconnect and clear old tokens, but Anthropic OAuth is not a @@ -470,6 +472,21 @@ quality. The standard Z.ai `/api/paas/` endpoint remains a `metered_api` route. Pulse uses model identifiers in the form: `provider:model-name` +Custom OpenAI-compatible model catalogs are authoritative. Pulse lists every +non-empty model ID returned by the endpoint, including IDs without a known +vendor prefix, and binds them to the configured `openai` provider instead of +guessing from the ID. Pulse omits empty Authorization headers, uses the +portable `max_tokens` request field, and omits optional OpenAI stream extensions +on custom endpoints. If an endpoint explicitly supports only buffered +completions, Pulse validates the complete response before projecting it through +the streaming runtime. Partial or malformed tool responses never become +executable calls. + +Ollama `keep_alive` is an optional provider setting. Blank is the default and +means Pulse omits the field so the Ollama server's own policy applies; explicit +duration, seconds, `-1`, and `0` values persist across restart and apply to both +streaming and non-streaming requests. + You can set separate models for: - Chat (`chat_model`) - Patrol (`patrol_model`) @@ -492,6 +509,19 @@ Changing the selected model, provider credentials or endpoint, or relevant timeout settings invalidates the cached result. Slow evaluations can be cancelled from the UI without replacing the last completed evidence. +Provider transport health is reported independently from Patrol capability. A +local endpoint can remain healthy and usable for ordinary Assistant chat while +the selected model receives an amber Patrol warning because it did not +demonstrate typed tool calls, context selection, continuation, or the required +latency envelope. That warning does not mislabel the provider as disconnected, +and it does not weaken Patrol's fail-closed tool/action admission. + +Removing a provider is a complete lifecycle action: Pulse deletes that +provider's stored credential, custom endpoint and provider-owned runtime +options, clears model selections routed through it, invalidates its model +catalog, and disables Pulse Intelligence when no provider remains. Credential +rotation through the legacy clear-key fields remains credential-specific. + ### Storage AI settings are stored encrypted at rest in `ai.enc` under the Pulse config directory. Related files: diff --git a/docs/release-control/v6/internal/subsystems/ai-runtime.md b/docs/release-control/v6/internal/subsystems/ai-runtime.md index ada7c6330..d718275a2 100644 --- a/docs/release-control/v6/internal/subsystems/ai-runtime.md +++ b/docs/release-control/v6/internal/subsystems/ai-runtime.md @@ -3517,6 +3517,16 @@ resolve canonical/source IDs and unique aliases before collection, reject accept providers that omit `[DONE]` only after a terminal `finish_reason`, but it must not emit `done` or executable tool calls from partial tool-call builders when the stream closes before that terminal provider state. + Custom compatible endpoints use portable `max_tokens`, omit the optional + `stream_options` extension, and omit Authorization when no key was saved. + A server that explicitly rejects streaming, or returns a complete + `application/json` completion for `stream=true`, may use the buffered + compatibility path only after the entire response and tool arguments have + validated; generic request failures, mid-stream failures, incomplete + responses, and malformed tools must not trigger or survive that fallback. + Restricted outbound transport must preserve metadata-service blocking, + DNS validation, and same-origin redirects while allowing operator-selected + private and loopback model servers. 16. Keep Patrol investigations product-facing through the shared `aicontracts.InvestigationRecord` contract. Patrol may keep `InvestigationSession` as execution detail, but Assistant handoff, @@ -6228,6 +6238,15 @@ alias-retirement posture and a recommendation to select the current V4 model IDs; unknown direct DeepSeek model IDs must be not-ready with `model_unavailable`; and known reasoning-only families must continue to fail closed before Patrol work is admitted. +The explicit Patrol model advisor must keep provider/model transport health +separate from Patrol capability evidence. Successful model-list connectivity +sets `transport_healthy`; only the typed streaming/buffered tool protocol, +context fixtures, continuation and latency checks set `patrol_capable`. +Ordinary Assistant success plus missing Patrol tools is a warning and +`not_suitable` Patrol mode, not a provider connection failure. Patrol action +admission remains fail closed. Local Ollama and custom OpenAI-compatible health, +catalog and preflight operations honor the configured request timeout so cold +model loads are not truncated by the hosted-provider 30-second check budget. That same browser-owned chat read model must keep target normalization helper- driven. Assistant shells may still derive legacy VM identifiers or display labels for read-only targeting, but they must do so through shared helpers and diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 359173864..a2664340e 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -3459,8 +3459,20 @@ successful targeted check from a queued response alone. The Ollama provider payload also owns `ollama_keep_alive` as the canonical request keep-alive field: GET and update responses must expose the normalized configured value, update requests must reject malformed values, - an empty string means omit Ollama `keep_alive`, and stored provider secrets - remain masked independently of that runtime option. + an empty string is the install default and means omit Ollama `keep_alive` + so the server policy is inherited, and stored provider secrets remain + masked independently of that runtime option. The shared provider registry + marks OpenAI as key-optional only when `openai_base_url` selects a custom + compatible endpoint; the official OpenAI route and other hosted providers + remain key-required. `configured_providers`, provider tests, and model + listing must accept that keyless custom route without emitting an empty + Authorization header. Custom model-list results retain every non-empty + opaque ID and carry server-authored `provider: "openai"` identity. + `remove_providers` is the complete provider lifecycle mutation: it removes + provider-owned secrets, endpoints and runtime options, clears selected + models for that provider, invalidates model inventory, and disables Pulse + Intelligence if the final provider is removed. Legacy `clear_*` fields + remain single-credential mutations. Discovery scheduling is part of that same AI settings payload contract: settings saves from `frontend-modern/src/components/Settings/useAISettingsState.ts` must send `discovery_enabled` and `discovery_interval_hours` together as diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index e193c070b..816b1c507 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -1568,6 +1568,16 @@ Agent`), with the plain-language source phrase available through accessible arrays/maps and the backend registry projection instead of introducing provider-specific JSX branches, local configured-state inference, or browser-owned default endpoint facts. + The OpenAI card must present its API key as optional for a custom compatible + endpoint and treat a saved base URL as configured provider state without + inventing model-family prefixes in the browser. Readiness presentation + consumes `transport_healthy` and `patrol_capable`: a reachable provider + whose model did not emit Patrol tools is amber and explicitly remains + usable for ordinary Assistant chat. Provider removal sends the complete + `remove_providers` lifecycle mutation and rehydrates endpoint, credential, + model, enabled state, and catalog from the response instead of clearing + only the visible credential input. A blank Ollama keep-alive control means + inherit the server default and must round-trip as blank. Local subscription-agent providers are the deliberate exception to credential inputs: their setup rows and first-run options render an explicit boolean opt-in, explain that Pulse uses an already authenticated diff --git a/frontend-modern/src/components/Settings/AIModelSelectionSection.tsx b/frontend-modern/src/components/Settings/AIModelSelectionSection.tsx index 8e331ea9b..a51d0aaa1 100644 --- a/frontend-modern/src/components/Settings/AIModelSelectionSection.tsx +++ b/frontend-modern/src/components/Settings/AIModelSelectionSection.tsx @@ -79,6 +79,7 @@ export const PatrolModelReadinessControl: Component<{ state: AISettingsState }> if (!r) return 'idle'; if (isStaleAgainstFormSelection()) return 'warning'; if (r.status === 'pass') return 'success'; + if (r.transport_healthy && !r.patrol_capable) return 'warning'; if (r.status === 'warning') return 'warning'; return 'error'; }; @@ -104,6 +105,8 @@ export const PatrolModelReadinessControl: Component<{ state: AISettingsState }> } if (r.max_verified_mode === 'approval') return 'Verified for Watch only and Ask first'; if (r.max_verified_mode === 'monitor') return 'Verified for Watch only'; + if (r.transport_healthy && !r.patrol_capable) + return 'Provider connected; Patrol capability not verified'; if (r.status === 'warning') return 'Patrol model needs attention'; return 'Patrol model not verified'; }; diff --git a/frontend-modern/src/components/Settings/AIProviderConfigurationSection.tsx b/frontend-modern/src/components/Settings/AIProviderConfigurationSection.tsx index f56f40713..6e0bca287 100644 --- a/frontend-modern/src/components/Settings/AIProviderConfigurationSection.tsx +++ b/frontend-modern/src/components/Settings/AIProviderConfigurationSection.tsx @@ -84,8 +84,8 @@ export const AIProviderConfigurationSection: Component

- Configure API keys for each provider you want to use. Models from all configured providers - will appear in the model selectors. + Configure API keys, subscription logins, or local/custom endpoints. Models returned by + every configured provider will appear in the model selectors.

@@ -200,6 +200,9 @@ export const AIProviderConfigurationSection: Component + + + - Clear + Remove diff --git a/frontend-modern/src/components/Settings/__tests__/AISettings.test.tsx b/frontend-modern/src/components/Settings/__tests__/AISettings.test.tsx index 862b23dda..dc48920c8 100644 --- a/frontend-modern/src/components/Settings/__tests__/AISettings.test.tsx +++ b/frontend-modern/src/components/Settings/__tests__/AISettings.test.tsx @@ -121,7 +121,7 @@ const baseSettings = (): AISettingsType => ({ gemini_configured: false, ollama_configured: false, ollama_base_url: 'http://localhost:11434', - ollama_keep_alive: '30s', + ollama_keep_alive: '', configured_providers: [], }); @@ -944,6 +944,77 @@ describe('AISettings Ollama provider options', () => { }); }); +describe('AISettings OpenAI-compatible provider lifecycle', () => { + beforeEach(() => { + resetAllMocks(); + setupDefaultMocks(); + }); + + afterEach(() => { + cleanup(); + }); + + it('saves a keyless custom endpoint as provider configuration', async () => { + updateSettingsMock.mockImplementation(async (payload: Record) => ({ + ...baseSettings(), + openai_configured: true, + configured: false, + openai_base_url: payload.openai_base_url as string, + configured_providers: ['openai'], + })); + + renderComponent(); + fireEvent.click(await screen.findByRole('button', { name: /openai/i })); + fireEvent.input(await screen.findByLabelText('OpenAI Custom Base URL'), { + target: { value: 'http://127.0.0.1:8080/v1' }, + }); + fireEvent.click(screen.getByRole('button', { name: /Save provider settings/i })); + + await waitFor(() => { + expect(updateSettingsMock).toHaveBeenCalledWith( + expect.objectContaining({ + openai_base_url: 'http://127.0.0.1:8080/v1', + }), + ); + }); + expect(updateSettingsMock.mock.calls[0]?.[0]).not.toHaveProperty('openai_api_key'); + }); + + it('removes endpoint, credentials, and selected models through the lifecycle API', async () => { + const configured = { + ...baseSettings(), + enabled: true, + configured: true, + model: 'openai:opaque-local-model', + chat_model: 'openai:opaque-local-model', + patrol_model: 'openai:opaque-local-model', + openai_configured: true, + openai_base_url: 'http://127.0.0.1:8080/v1', + configured_providers: ['openai' as const], + }; + getSettingsMock.mockResolvedValue(configured); + updateSettingsMock.mockResolvedValue({ + ...baseSettings(), + enabled: false, + model: '', + openai_base_url: '', + }); + const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true); + + renderComponent(); + fireEvent.click(await screen.findByRole('button', { name: /openai/i })); + fireEvent.click(await screen.findByRole('button', { name: 'Remove' })); + + await waitFor(() => { + expect(updateSettingsMock).toHaveBeenCalledWith({ + remove_providers: ['openai'], + }); + }); + expect(confirmSpy).toHaveBeenCalledWith(expect.stringContaining('only configured provider')); + confirmSpy.mockRestore(); + }); +}); + describe('AISettings provider save failure context', () => { beforeEach(() => { resetAllMocks(); diff --git a/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts b/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts index bee0dc085..beb3893f9 100644 --- a/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts +++ b/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts @@ -770,7 +770,6 @@ describe('settings architecture guardrails', () => { for (const provider of ['zai', 'groq', 'mistral', 'cerebras', 'together', 'fireworks']) { expect(aiSettingsModelSource).toContain(`provider: '${provider}'`); expect(aiSettingsStateSource).toContain(`${provider}_api_key`); - expect(aiSettingsStateSource).toContain(`clear_${provider}_key`); expect(aiSettingsStateSource).toContain(`${provider}_configured`); } expect(aiSettingsModelSource).toContain('extraFields: ['); @@ -811,6 +810,16 @@ describe('settings architecture guardrails', () => { expect(aiSettingsModelSource).not.toContain('qwen'); }); + it('keeps local provider inheritance and removal on explicit lifecycle contracts', () => { + expect(aiSettingsStateSource).toContain("data.ollama_keep_alive ?? ''"); + expect(aiSettingsStateSource).toContain('remove_providers: [provider]'); + expect(aiSettingsStateSource).not.toContain('clearPayload.clear_openai_key'); + expect(aiSettingsModelSource).toContain('API key (optional for a custom endpoint)'); + expect(aiSettingsModelSource).toContain( + 'Leave the API key blank when the endpoint does not require one.', + ); + }); + it('keeps Assistant session maintenance limited to Pulse-owned session actions', () => { expect(aiChatMaintenanceSectionSource).toContain('Summarize session'); expect(aiChatMaintenanceSectionSource).toContain('handleSessionSummarize'); diff --git a/frontend-modern/src/components/Settings/aiSettingsModel.ts b/frontend-modern/src/components/Settings/aiSettingsModel.ts index 5f507b7e5..49569ca8c 100644 --- a/frontend-modern/src/components/Settings/aiSettingsModel.ts +++ b/frontend-modern/src/components/Settings/aiSettingsModel.ts @@ -34,6 +34,7 @@ export type AIProviderConfig = { configuredLabel?: string; inputType: 'password' | 'url' | 'toggle'; inputField: keyof AIProviderCredentialsFormState; + inputLabel?: string; placeholder: string; configuredPlaceholder?: string; actionLinkLabel: string; @@ -118,11 +119,12 @@ export const AI_PROVIDER_CONFIGS: AIProviderConfig[] = [ configuredLabel: 'Configured', inputType: 'password', inputField: 'openaiApiKey', + inputLabel: 'API key (optional for a custom endpoint)', placeholder: 'sk-...', configuredPlaceholder: '••••••••••• (configured)', actionLinkLabel: 'Get API key →', actionLinkHref: 'https://platform.openai.com/api-keys', - clearTitle: 'Clear API key', + clearTitle: 'Remove OpenAI provider', extraFields: [ { label: 'Custom Base URL', @@ -130,6 +132,8 @@ export const AI_PROVIDER_CONFIGS: AIProviderConfig[] = [ inputField: 'openaiBaseUrl', placeholder: 'https://api.together.xyz/v1 (optional)', type: 'url', + helperText: + 'Use llama.cpp, LocalAI, LM Studio, or another OpenAI-compatible endpoint. Leave the API key blank when the endpoint does not require one.', }, ], }, diff --git a/frontend-modern/src/components/Settings/useAISettingsState.ts b/frontend-modern/src/components/Settings/useAISettingsState.ts index 4d1da774f..e87d929c5 100644 --- a/frontend-modern/src/components/Settings/useAISettingsState.ts +++ b/frontend-modern/src/components/Settings/useAISettingsState.ts @@ -307,7 +307,7 @@ export const useAISettingsState = (options: AISettingsStateOptions = {}) => { fireworksApiKey: '', geminiApiKey: '', ollamaBaseUrl: 'http://localhost:11434', - ollamaKeepAlive: '30s', + ollamaKeepAlive: '', openaiBaseUrl: '', zaiBaseUrl: '', codexSubscriptionEnabled: false, @@ -405,7 +405,7 @@ export const useAISettingsState = (options: AISettingsStateOptions = {}) => { fireworksApiKey: '', geminiApiKey: '', ollamaBaseUrl: 'http://localhost:11434', - ollamaKeepAlive: '30s', + ollamaKeepAlive: '', openaiBaseUrl: '', zaiBaseUrl: '', codexSubscriptionEnabled: false, @@ -453,7 +453,7 @@ export const useAISettingsState = (options: AISettingsStateOptions = {}) => { fireworksApiKey: '', geminiApiKey: '', ollamaBaseUrl: data.ollama_base_url || 'http://localhost:11434', - ollamaKeepAlive: data.ollama_keep_alive ?? '30s', + ollamaKeepAlive: data.ollama_keep_alive ?? '', openaiBaseUrl: data.openai_base_url || '', zaiBaseUrl: data.zai_base_url || '', codexSubscriptionEnabled: Boolean(data.codex_subscription_enabled), @@ -694,9 +694,7 @@ export const useAISettingsState = (options: AISettingsStateOptions = {}) => { setPatrolModelReadinessResult(result); } catch (error) { const errorName = - typeof error === 'object' && error !== null && 'name' in error - ? String(error.name) - : ''; + typeof error === 'object' && error !== null && 'name' in error ? String(error.name) : ''; if (errorName === 'AbortError') { notificationStore.info('Patrol model evaluation cancelled.'); return; @@ -934,7 +932,7 @@ export const useAISettingsState = (options: AISettingsStateOptions = {}) => { if (!isAIProviderConfigured(modelProvider, settings())) { const isAddingCredential = (modelProvider === 'anthropic' && form.anthropicApiKey.trim()) || - (modelProvider === 'openai' && form.openaiApiKey.trim()) || + (modelProvider === 'openai' && (form.openaiApiKey.trim() || form.openaiBaseUrl.trim())) || (modelProvider === 'openrouter' && form.openrouterApiKey.trim()) || (modelProvider === 'deepseek' && form.deepseekApiKey.trim()) || (modelProvider === 'zai' && form.zaiApiKey.trim()) || @@ -1057,7 +1055,7 @@ export const useAISettingsState = (options: AISettingsStateOptions = {}) => { ) { payload.ollama_base_url = form.ollamaBaseUrl.trim(); } - if (form.ollamaKeepAlive.trim() !== (settings()?.ollama_keep_alive ?? '30s')) { + if (form.ollamaKeepAlive.trim() !== (settings()?.ollama_keep_alive ?? '')) { payload.ollama_keep_alive = form.ollamaKeepAlive.trim(); } if (form.openaiBaseUrl !== (settings()?.openai_base_url || '')) { @@ -1231,14 +1229,14 @@ export const useAISettingsState = (options: AISettingsStateOptions = {}) => { const currentModel = form.model.trim(); const modelUsesProvider = currentModel && getProviderFromModelId(currentModel) === provider; - let confirmMessage = `Clear ${getAIProviderDisplayName(provider) || provider} credentials?`; + let confirmMessage = `Remove ${getAIProviderDisplayName(provider) || provider} and its saved settings?`; if (isLastProvider) { confirmMessage = - 'Warning: this is your only configured provider. Clearing it will disable Pulse Assistant until you configure another provider. Continue?'; + 'Warning: this is your only configured provider. Removing it will disable Pulse Assistant and clear its selected models until you configure another provider. Continue?'; } else if (modelUsesProvider) { - confirmMessage = `Your current model uses ${getAIProviderDisplayName(provider) || provider}. Clearing this will require selecting a different model. Continue?`; + confirmMessage = `Your current model uses ${getAIProviderDisplayName(provider) || provider}. Removing it will clear provider-owned model selections. Continue?`; } else { - confirmMessage += " You'll need to re-enter credentials to use this provider."; + confirmMessage += " You'll need to configure it again to use this provider."; } if (!confirm(confirmMessage)) { @@ -1247,48 +1245,18 @@ export const useAISettingsState = (options: AISettingsStateOptions = {}) => { setSaving(true); try { - const clearPayload: Record = {}; - if (provider === 'anthropic') clearPayload.clear_anthropic_key = true; - if (provider === 'openai') clearPayload.clear_openai_key = true; - if (provider === 'openrouter') clearPayload.clear_openrouter_key = true; - if (provider === 'deepseek') clearPayload.clear_deepseek_key = true; - if (provider === 'zai') clearPayload.clear_zai_key = true; - if (provider === 'groq') clearPayload.clear_groq_key = true; - if (provider === 'mistral') clearPayload.clear_mistral_key = true; - if (provider === 'cerebras') clearPayload.clear_cerebras_key = true; - if (provider === 'together') clearPayload.clear_together_key = true; - if (provider === 'fireworks') clearPayload.clear_fireworks_key = true; - if (provider === 'gemini') clearPayload.clear_gemini_key = true; - if (provider === 'ollama') clearPayload.clear_ollama_url = true; - if (provider === 'codex-subscription') clearPayload.codex_subscription_enabled = false; - if (provider === 'claude-subscription') clearPayload.claude_subscription_enabled = false; - - await AIAPI.updateSettings(clearPayload); - const newSettings = await AIAPI.getSettings(); + const newSettings = await AIAPI.updateSettings({ remove_providers: [provider] }); setSettings(newSettings); + resetForm(newSettings); syncModelCatalogForSettings(newSettings); void runProviderPreflight(newSettings); - if (provider === 'anthropic') setForm('anthropicApiKey', ''); - if (provider === 'openai') setForm('openaiApiKey', ''); - if (provider === 'openrouter') setForm('openrouterApiKey', ''); - if (provider === 'deepseek') setForm('deepseekApiKey', ''); - if (provider === 'zai') setForm('zaiApiKey', ''); - if (provider === 'groq') setForm('groqApiKey', ''); - if (provider === 'mistral') setForm('mistralApiKey', ''); - if (provider === 'cerebras') setForm('cerebrasApiKey', ''); - if (provider === 'together') setForm('togetherApiKey', ''); - if (provider === 'fireworks') setForm('fireworksApiKey', ''); - if (provider === 'gemini') setForm('geminiApiKey', ''); - if (provider === 'ollama') setForm('ollamaBaseUrl', ''); - if (provider === 'codex-subscription') setForm('codexSubscriptionEnabled', false); - if (provider === 'claude-subscription') setForm('claudeSubscriptionEnabled', false); - - notificationStore.success(`${provider} credentials cleared`); + notificationStore.success(`${getAIProviderDisplayName(provider) || provider} removed`); } catch (error) { - logger.error(`[AISettings] Clear ${provider} failed:`, error); + logger.error(`[AISettings] Remove ${provider} failed:`, error); + const detail = error instanceof Error ? error.message.trim() : ''; notificationStore.error( - getAICredentialsClearErrorMessage(error instanceof Error ? error.message : ''), + detail ? getAICredentialsClearErrorMessage(detail) : 'Unable to remove provider.', ); } finally { setSaving(false); diff --git a/frontend-modern/src/types/ai.ts b/frontend-modern/src/types/ai.ts index 5075eb405..380c70094 100644 --- a/frontend-modern/src/types/ai.ts +++ b/frontend-modern/src/types/ai.ts @@ -30,6 +30,7 @@ export interface AIProviderDefinition { clear_key_field?: string; base_url_field?: string; requires_api_key: boolean; + api_key_optional_with_custom_base_url?: boolean; user_configurable: boolean; gateway: boolean; configured: boolean; @@ -99,7 +100,7 @@ export interface AISettings { patrol_auto_fix?: boolean; // true if Patrol can remediate without approval // Multi-provider configuration anthropic_configured: boolean; // true if Anthropic API key is set - openai_configured: boolean; // true if OpenAI API key is set + openai_configured: boolean; // true if an OpenAI key or custom compatible endpoint is set openrouter_configured: boolean; // true if OpenRouter API key is set deepseek_configured: boolean; // true if DeepSeek API key is set gemini_configured: boolean; // true if Gemini API key is set @@ -182,6 +183,8 @@ export interface PatrolModeSuitability { export interface PatrolModelReadinessSnapshot { probe_version: string; success: boolean; + transport_healthy?: boolean; + patrol_capable?: boolean; status: PatrolModelReadinessStatus; provider?: string; model?: string; @@ -265,6 +268,7 @@ export interface AISettingsUpdateRequest { clear_together_key?: boolean; // Clear Together AI API key clear_fireworks_key?: boolean; // Clear Fireworks AI API key clear_ollama_url?: boolean; // Clear Ollama URL + remove_providers?: AIProvider[]; // Remove provider-owned credentials, endpoint, options, and model selections // Cost controls cost_budget_usd_30d?: number; diff --git a/internal/ai/openai_compatible_lifecycle_test.go b/internal/ai/openai_compatible_lifecycle_test.go new file mode 100644 index 000000000..532e01e4c --- /dev/null +++ b/internal/ai/openai_compatible_lifecycle_test.go @@ -0,0 +1,73 @@ +package ai + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" +) + +func TestKeylessOpenAICompatibleModelCatalogAndProviderRemovalLifecycle(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get("Authorization"); got != "" { + t.Fatalf("keyless endpoint received Authorization %q", got) + } + if r.URL.Path != "/v1/models" { + http.NotFound(w, r) + return + } + _ = json.NewEncoder(w).Encode(map[string]interface{}{ + "data": []map[string]interface{}{ + {"id": "model-without-a-known-prefix", "name": "Opaque local model"}, + {"id": "vendor/model:quant", "name": "Vendor model"}, + }, + }) + })) + defer server.Close() + + dir := t.TempDir() + persistence := config.NewConfigPersistence(dir) + cfg := config.NewDefaultAIConfig() + cfg.Enabled = true + cfg.Model = "openai:model-without-a-known-prefix" + cfg.OpenAIBaseURL = server.URL + if err := persistence.SaveAIConfig(*cfg); err != nil { + t.Fatalf("SaveAIConfig() error = %v", err) + } + + service := NewService(persistence, nil) + models, cached, err := service.ListModelsWithCache(context.Background()) + if err != nil { + t.Fatalf("ListModelsWithCache() error = %v", err) + } + if cached || len(models) != 2 { + t.Fatalf("models=%+v cached=%t, want two fresh models", models, cached) + } + if models[0].ID != "openai:model-without-a-known-prefix" || models[0].Provider != config.AIProviderOpenAI { + t.Fatalf("opaque model lost authoritative provider identity: %+v", models[0]) + } + if models[1].ID != "openai:vendor/model:quant" || models[1].Provider != config.AIProviderOpenAI { + t.Fatalf("arbitrary model id was misclassified: %+v", models[1]) + } + initialCacheKey := service.modelsCache.key + + if err := cfg.RemoveProvider(config.AIProviderOpenAI); err != nil { + t.Fatalf("RemoveProvider() error = %v", err) + } + if err := persistence.SaveAIConfig(*cfg); err != nil { + t.Fatalf("SaveAIConfig(removed) error = %v", err) + } + models, cached, err = service.ListModelsWithCache(context.Background()) + if err != nil { + t.Fatalf("ListModelsWithCache(after removal) error = %v", err) + } + if len(models) != 0 { + t.Fatalf("removed provider models survived cache invalidation: %+v cached=%t", models, cached) + } + if service.modelsCache.key == initialCacheKey { + t.Fatal("provider removal did not invalidate the model catalog cache key") + } +} diff --git a/internal/ai/patrol_model_readiness.go b/internal/ai/patrol_model_readiness.go index 2c5f362f1..8ba1b014d 100644 --- a/internal/ai/patrol_model_readiness.go +++ b/internal/ai/patrol_model_readiness.go @@ -91,23 +91,28 @@ type PatrolModelReadinessMetadata struct { // operator-triggered advisor run. CacheKey is local-only invalidation state and // is intentionally excluded from every API response. type PatrolModelReadinessResult struct { - ProbeVersion string `json:"probe_version"` - Success bool `json:"success"` - Status string `json:"status"` - Provider string `json:"provider,omitempty"` - Model string `json:"model,omitempty"` - DurationMs int64 `json:"duration_ms"` - MaxVerifiedMode string `json:"max_verified_mode,omitempty"` - Cause PatrolFailureCause `json:"cause,omitempty"` - Summary string `json:"summary"` - Recommendation string `json:"recommendation,omitempty"` - Metadata *PatrolModelReadinessMetadata `json:"metadata,omitempty"` - Dimensions PatrolModelReadinessDimensions `json:"dimensions"` - Modes PatrolModelReadinessModes `json:"modes"` - CacheKey string `json:"-"` - inputTokens int - outputTokens int - providerCalls int + ProbeVersion string `json:"probe_version"` + Success bool `json:"success"` + // TransportHealthy records provider/model reachability independently from + // PatrolCapable. A model can serve ordinary Assistant chat while failing + // Patrol's stricter streaming tool protocol. + TransportHealthy bool `json:"transport_healthy"` + PatrolCapable bool `json:"patrol_capable"` + Status string `json:"status"` + Provider string `json:"provider,omitempty"` + Model string `json:"model,omitempty"` + DurationMs int64 `json:"duration_ms"` + MaxVerifiedMode string `json:"max_verified_mode,omitempty"` + Cause PatrolFailureCause `json:"cause,omitempty"` + Summary string `json:"summary"` + Recommendation string `json:"recommendation,omitempty"` + Metadata *PatrolModelReadinessMetadata `json:"metadata,omitempty"` + Dimensions PatrolModelReadinessDimensions `json:"dimensions"` + Modes PatrolModelReadinessModes `json:"modes"` + CacheKey string `json:"-"` + inputTokens int + outputTokens int + providerCalls int } type patrolModelReadinessCache struct { @@ -373,6 +378,8 @@ func (s *Service) RunPatrolModelReadiness(ctx context.Context, providerName, mod result.Provider = DemoPatrolProvider result.Model = DemoPatrolModel result.Success = true + result.TransportHealthy = true + result.PatrolCapable = true result.Status = PatrolModelReadinessPass result.MaxVerifiedMode = config.PatrolAutonomyApproval result.Summary = "Demo mode simulated a successful Patrol readiness evaluation." @@ -476,13 +483,14 @@ func runPatrolModelReadinessWithProvider(ctx context.Context, cfg *config.AIConf Summary: connectionSummary, DurationMs: time.Since(connectionStarted).Milliseconds(), } + result.TransportHealthy = true streamingProvider, ok := provider.(providers.StreamingProvider) if !ok { result.Cause = PatrolFailureCauseModelUnsupportedTools - result.Status = PatrolModelReadinessFail - result.Summary = "The selected provider does not expose Patrol's streaming transport." - result.Recommendation = "Choose a provider and model that support streaming tool calls." + result.Status = PatrolModelReadinessWarning + result.Summary = "The provider is healthy for ordinary chat, but this route does not expose Patrol's streaming transport." + result.Recommendation = "Use this provider for ordinary Assistant chat, or choose a Patrol route with streaming tool-call support." result.Dimensions.ToolProtocol = PatrolModelReadinessDimension{Status: PatrolModelReadinessFail, Summary: result.Summary} result.Modes.Monitor = PatrolModeSuitability{Status: PatrolModeNotSuitable, Summary: result.Summary} result.Modes.Approval = PatrolModeSuitability{Status: PatrolModeNotSuitable, Summary: result.Summary} @@ -665,10 +673,11 @@ func runPatrolModelReadinessWithProvider(ctx context.Context, cfg *config.AIConf } result.Success = watchVerified - result.Status = PatrolModelReadinessFail + result.PatrolCapable = watchVerified + result.Status = PatrolModelReadinessWarning result.Cause = PatrolFailureCauseModelToolSupportUnverified - result.Summary = "The selected model did not pass Patrol's tool protocol evaluation." - result.Recommendation = "Choose a model with reliable streaming tool use, or lower the model's workload and retry." + result.Summary = "The provider is healthy for ordinary chat, but the selected model did not demonstrate Patrol's streaming tool protocol." + result.Recommendation = "Keep using this route for ordinary Assistant chat, or choose a Patrol model with reliable streaming tool use." if contextStatus == PatrolModelReadinessFail && toolStatus == PatrolModelReadinessPass { result.Cause = PatrolFailureCauseContextQualityFailed result.Summary = "The selected model did not pass Patrol's context-quality evaluation." @@ -683,6 +692,7 @@ func runPatrolModelReadinessWithProvider(ctx context.Context, cfg *config.AIConf if watchVerified { result.Status = PatrolModelReadinessPass result.Cause = PatrolFailureCauseNone + result.PatrolCapable = true result.MaxVerifiedMode = config.PatrolAutonomyMonitor result.Summary = "Verified for Watch only on this install." result.Recommendation = "Keep Safe auto-fix and Autopilot disabled until an extended governed canary has passed." diff --git a/internal/ai/patrol_model_readiness_test.go b/internal/ai/patrol_model_readiness_test.go index 4e49753af..12cc3c72d 100644 --- a/internal/ai/patrol_model_readiness_test.go +++ b/internal/ai/patrol_model_readiness_test.go @@ -4,6 +4,8 @@ import ( "context" "encoding/json" "errors" + "net/http" + "net/http/httptest" "os" "path/filepath" "regexp" @@ -124,7 +126,7 @@ func TestRunPatrolModelReadinessWithProvider_VerifiesMonitorAndApproval(t *testi context.Background(), readinessTestConfig(), config.AIProviderOllama, "test-model", "ollama:test-model", provider, ) - if !result.Success || result.Status != PatrolModelReadinessPass { + if !result.Success || result.Status != PatrolModelReadinessPass || !result.TransportHealthy || !result.PatrolCapable { t.Fatalf("expected successful readiness evaluation, got %+v", result) } if result.MaxVerifiedMode != config.PatrolAutonomyApproval { @@ -182,7 +184,7 @@ func TestRunPatrolModelReadinessWithProvider_SeparatesProtocolFromContextQuality } } -func TestRunPatrolModelReadinessWithProvider_ProtocolFailureIsHardFailure(t *testing.T) { +func TestRunPatrolModelReadinessWithProvider_ProtocolFailureIsCapabilityWarning(t *testing.T) { provider := &scriptedReadinessProvider{contextWindow: 32768, wrongProtocol: true} result := runPatrolModelReadinessWithProvider( context.Background(), readinessTestConfig(), config.AIProviderOllama, "test-model", "ollama:test-model", provider, @@ -191,11 +193,67 @@ func TestRunPatrolModelReadinessWithProvider_ProtocolFailureIsHardFailure(t *tes if result.Dimensions.ToolProtocol.Passed != 0 || result.Success { t.Fatalf("expected hard protocol failure, got %+v", result) } + if !result.TransportHealthy || result.PatrolCapable || result.Status != PatrolModelReadinessWarning { + t.Fatalf("provider health must remain distinct from Patrol capability, got %+v", result) + } if result.Modes.Monitor.Status != PatrolModeNotSuitable { t.Fatalf("Watch only should be unsuitable, got %+v", result.Modes.Monitor) } } +func TestRunPatrolModelReadinessWithProvider_ReproducesHealthyAssistantButMissingPatrolTools(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/v1/models": + _, _ = w.Write([]byte(`{"data":[{"id":"opaque-local-model"}]}`)) + case "/v1/chat/completions": + var request map[string]interface{} + if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + t.Errorf("decode request: %v", err) + return + } + if request["stream"] == true { + w.Header().Set("Content-Type", "text/event-stream") + _, _ = w.Write([]byte("data: {\"choices\":[{\"delta\":{\"content\":\"plain text only\"}}]}\n\ndata: {\"choices\":[{\"delta\":{},\"finish_reason\":\"stop\"}]}\n\ndata: [DONE]\n\n")) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"model":"opaque-local-model","choices":[{"message":{"role":"assistant","content":"Assistant works"},"finish_reason":"stop"}]}`)) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + cfg := &config.AIConfig{ + Enabled: true, + Model: "openai:opaque-local-model", + PatrolModel: "openai:opaque-local-model", + OpenAIBaseURL: server.URL, + RequestTimeoutSeconds: 2, + } + provider := providers.NewOpenAICompatibleClient("openai", "", "opaque-local-model", server.URL, 2*time.Second) + assistantResponse, err := provider.Chat(context.Background(), providers.ChatRequest{ + Messages: []providers.Message{{Role: "user", Content: "hello"}}, + }) + if err != nil || assistantResponse.Content != "Assistant works" { + t.Fatalf("ordinary Assistant chat failed: response=%+v err=%v", assistantResponse, err) + } + + result := runPatrolModelReadinessWithProvider( + context.Background(), cfg, config.AIProviderOpenAI, "opaque-local-model", "openai:opaque-local-model", provider, + ) + if !result.TransportHealthy || result.PatrolCapable || result.Success { + t.Fatalf("transport and Patrol capability were not separated: %+v", result) + } + if result.Status != PatrolModelReadinessWarning || result.Dimensions.Connectivity.Status != PatrolModelReadinessPass { + t.Fatalf("healthy provider should remain a warning, not a connection failure: %+v", result) + } + if result.Dimensions.ToolProtocol.Passed != 0 || result.Cause != PatrolFailureCauseModelToolSupportUnverified { + t.Fatalf("missing tool calls should be reported as an unverified Patrol capability: %+v", result) + } +} + func TestRunPatrolModelReadinessWithProvider_ContinuationCapsAtMonitor(t *testing.T) { provider := &scriptedReadinessProvider{contextWindow: 32768, skipContinuation: true} result := runPatrolModelReadinessWithProvider( diff --git a/internal/ai/patrol_preflight.go b/internal/ai/patrol_preflight.go index 9782ce40e..76212cda3 100644 --- a/internal/ai/patrol_preflight.go +++ b/internal/ai/patrol_preflight.go @@ -44,13 +44,21 @@ const patrolPreflightToolName = "verify_pulse_patrol" const defaultPatrolPreflightTimeout = 30 * time.Second -func patrolPreflightTimeout(provider string) time.Duration { +func patrolPreflightTimeout(provider string, cfg *config.AIConfig) time.Duration { switch strings.TrimSpace(provider) { case config.AIProviderCodexSubscription, config.AIProviderClaudeSubscription: return providers.SubscriptionAgentMinimumRequestTimeout + case config.AIProviderOllama: + if cfg != nil && cfg.GetRequestTimeout() > defaultPatrolPreflightTimeout { + return cfg.GetRequestTimeout() + } + case config.AIProviderOpenAI: + if cfg != nil && config.IsCustomOpenAICompatibleEndpoint(cfg.OpenAIBaseURL) && cfg.GetRequestTimeout() > defaultPatrolPreflightTimeout { + return cfg.GetRequestTimeout() + } default: - return defaultPatrolPreflightTimeout } + return defaultPatrolPreflightTimeout } // patrolPreflightCache holds the most recent PatrolPreflightResult plus @@ -184,7 +192,7 @@ func (s *Service) RunPatrolToolPreflight(ctx context.Context, providerName, mode parsedProvider, parsedModel := config.ParseModelString(modelStr) result.Provider = parsedProvider result.Model = parsedModel - preflightCtx, cancel := context.WithTimeout(ctx, patrolPreflightTimeout(parsedProvider)) + preflightCtx, cancel := context.WithTimeout(ctx, patrolPreflightTimeout(parsedProvider, cfg)) defer cancel() provider, err := providers.NewForModel(cfg, modelStr) diff --git a/internal/ai/patrol_preflight_test.go b/internal/ai/patrol_preflight_test.go index 502584cf8..25ec65a76 100644 --- a/internal/ai/patrol_preflight_test.go +++ b/internal/ai/patrol_preflight_test.go @@ -50,9 +50,12 @@ func newPatrolPreflightTestService(t *testing.T, model string, handler http.Hand func TestPatrolPreflightTimeoutIsRouteAware(t *testing.T) { tests := []struct { provider string + cfg *config.AIConfig want time.Duration }{ - {provider: config.AIProviderOpenAI, want: 30 * time.Second}, + {provider: config.AIProviderOpenAI, cfg: &config.AIConfig{OpenAIBaseURL: "http://localhost:8080/v1", RequestTimeoutSeconds: 180}, want: 3 * time.Minute}, + {provider: config.AIProviderOpenAI, cfg: &config.AIConfig{RequestTimeoutSeconds: 180}, want: 30 * time.Second}, + {provider: config.AIProviderOllama, cfg: &config.AIConfig{RequestTimeoutSeconds: 180}, want: 3 * time.Minute}, {provider: config.AIProviderAnthropic, want: 30 * time.Second}, {provider: config.AIProviderCodexSubscription, want: 2 * time.Minute}, {provider: config.AIProviderClaudeSubscription, want: 2 * time.Minute}, @@ -61,7 +64,7 @@ func TestPatrolPreflightTimeoutIsRouteAware(t *testing.T) { } for _, tt := range tests { t.Run(tt.provider, func(t *testing.T) { - if got := patrolPreflightTimeout(tt.provider); got != tt.want { + if got := patrolPreflightTimeout(tt.provider, tt.cfg); got != tt.want { t.Fatalf("patrolPreflightTimeout(%q) = %s, want %s", tt.provider, got, tt.want) } }) diff --git a/internal/ai/providers/factory.go b/internal/ai/providers/factory.go index c6fbfe978..70607912b 100644 --- a/internal/ai/providers/factory.go +++ b/internal/ai/providers/factory.go @@ -40,7 +40,7 @@ func NewForProvider(cfg *config.AIConfig, provider, model string) (Provider, err if config.IsOpenAICompatibleProvider(provider) { apiKey := cfg.GetAPIKeyForProvider(provider) - if apiKey == "" { + if apiKey == "" && cfg.ProviderRequiresAPIKey(provider) { return nil, fmt.Errorf("%s API key not configured", config.AIProviderDisplayName(provider)) } baseURL := cfg.GetBaseURLForProvider(provider) diff --git a/internal/ai/providers/ollama.go b/internal/ai/providers/ollama.go index c86345a09..77e854013 100644 --- a/internal/ai/providers/ollama.go +++ b/internal/ai/providers/ollama.go @@ -88,20 +88,13 @@ func normalizeOllamaBaseURL(raw string) (string, error) { func newOllamaHTTPClient(timeout time.Duration, streaming bool) *http.Client { clientTimeout := timeout + options := ollamaOutboundHTTPOptions if streaming { clientTimeout = 0 + options.ResponseHeaderTimeout = timeout } - client := securityutil.NewRestrictedOutboundHTTPClient(clientTimeout, ollamaOutboundHTTPOptions) - if streaming { - if transport, ok := client.Transport.(*http.Transport); ok { - // Streaming relies on context cancellation instead of a full client timeout, - // but it should still fail if the server never starts responding. - transport.ResponseHeaderTimeout = timeout - } - } - - return client + return securityutil.NewRestrictedOutboundHTTPClient(clientTimeout, options) } func (c *OllamaClient) applyAuth(req *http.Request) { diff --git a/internal/ai/providers/ollama_test.go b/internal/ai/providers/ollama_test.go index 2278f29a8..4ead6ab96 100644 --- a/internal/ai/providers/ollama_test.go +++ b/internal/ai/providers/ollama_test.go @@ -8,7 +8,6 @@ import ( "testing" "time" - "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -37,10 +36,8 @@ func TestOllamaClient_ChatStream_Success(t *testing.T) { assert.True(t, req.Stream) assert.Equal(t, "llama3", req.Model) assert.NotEmpty(t, req.Messages) - // #1425: Pulse must pass keep_alive so the model unloads shortly - // after the request burst ends instead of refreshing Ollama's - // 5-minute default TTL on every call. - assert.Equal(t, config.DefaultOllamaKeepAlive, req.KeepAlive) + // The default inherits the server policy, so keep_alive is omitted. + assert.Nil(t, req.KeepAlive) w.Header().Set("Content-Type", "application/x-ndjson") w.WriteHeader(http.StatusOK) @@ -227,8 +224,7 @@ func TestOllamaClient_Chat_Success(t *testing.T) { require.NoError(t, json.NewDecoder(r.Body).Decode(&req)) assert.False(t, req.Stream) assert.Equal(t, "llama3", req.Model) - // #1425: keep_alive must be set on non-streaming Chat too. - assert.Equal(t, config.DefaultOllamaKeepAlive, req.KeepAlive) + assert.Nil(t, req.KeepAlive) require.Len(t, req.Tools, 1) assert.Equal(t, "function", req.Tools[0].Type) assert.Equal(t, "get_time", req.Tools[0].Function.Name) @@ -297,6 +293,31 @@ func TestOllamaClient_Chat_UsesConfiguredKeepAlive(t *testing.T) { require.NoError(t, err) } +func TestOllamaClient_ChatStream_UsesConfiguredKeepAlive(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var req ollamaRequest + require.NoError(t, json.NewDecoder(r.Body).Decode(&req)) + assert.True(t, req.Stream) + assert.Equal(t, "24h", req.KeepAlive) + + w.Header().Set("Content-Type", "application/x-ndjson") + _ = json.NewEncoder(w).Encode(ollamaResponse{ + Model: "llama3", + Message: ollamaMessageResp{Role: "assistant", Content: "Hello"}, + Done: true, + }) + })) + defer server.Close() + + client, err := NewOllamaClientWithKeepAlive("llama3", server.URL, "", "", "24h", 0) + require.NoError(t, err) + + err = client.ChatStream(context.Background(), ChatRequest{ + Messages: []Message{{Role: "user", Content: "Hi"}}, + }, func(StreamEvent) {}) + require.NoError(t, err) +} + func TestOllamaClient_Chat_OmitsKeepAliveForServerDefault(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { var raw map[string]json.RawMessage diff --git a/internal/ai/providers/openai.go b/internal/ai/providers/openai.go index a476b1401..49483d0bc 100644 --- a/internal/ai/providers/openai.go +++ b/internal/ai/providers/openai.go @@ -13,6 +13,7 @@ import ( "time" "github.com/rcourtman/pulse-go-rewrite/internal/agentcapabilities" + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" "github.com/rs/zerolog/log" ) @@ -59,6 +60,12 @@ type OpenAIClient struct { streamFirstChunkTimeout time.Duration } +var openAICompatibleOutboundHTTPOptions = securityutil.RestrictedOutboundHTTPOptions{ + AllowedSchemes: []string{"http", "https"}, + AllowPrivateIPs: true, + AllowLoopback: true, +} + // NewOpenAIClient creates a new OpenAI API client // timeout is optional - pass 0 to use the default 5 minute timeout func NewOpenAIClient(apiKey, model, baseURL string, timeout time.Duration) *OpenAIClient { @@ -85,13 +92,23 @@ func NewOpenAICompatibleClient(providerName, apiKey, model, baseURL string, time apiKey: apiKey, model: model, baseURL: baseURL, - client: &http.Client{Timeout: timeout}, + client: newOpenAICompatibleHTTPClient(timeout, false), streamClient: newOpenAIStreamHTTPClient(timeout), streamChunkTimeout: boundedOpenAIStreamChunkTimeout(timeout), streamFirstChunkTimeout: timeout, } } +func newOpenAICompatibleHTTPClient(timeout time.Duration, streaming bool) *http.Client { + options := openAICompatibleOutboundHTTPOptions + clientTimeout := timeout + if streaming { + clientTimeout = 0 + options.ResponseHeaderTimeout = timeout + } + return securityutil.NewRestrictedOutboundHTTPClient(clientTimeout, options) +} + func normalizeOpenAICompatibleChatURL(baseURL string) string { baseURL = strings.TrimSpace(baseURL) if baseURL == "" { @@ -147,17 +164,9 @@ func stripOpenAICompatibleProviderPrefix(providerName, model string) string { } func newOpenAIStreamHTTPClient(timeout time.Duration) *http.Client { - client := &http.Client{} - if transport, ok := http.DefaultTransport.(*http.Transport); ok { - transport = transport.Clone() - // Local backends can hold the response headers while the model loads - // (Ollama does exactly this on a cold model), so the header wait - // honors the configured request timeout like the first-chunk wait - // does, rather than a short fixed bound. - transport.ResponseHeaderTimeout = timeout - client.Transport = transport - } - return client + // Local backends can hold response headers while the model loads. Bound + // that startup wait, but do not impose an overall timeout on a live stream. + return newOpenAICompatibleHTTPClient(timeout, true) } func boundedOpenAIStreamChunkTimeout(timeout time.Duration) time.Duration { @@ -314,6 +323,84 @@ type openaiErrorDetail struct { Code string `json:"code"` } +func convertOpenAIResponse(openaiResp openaiResponse) (*ChatResponse, error) { + if len(openaiResp.Choices) == 0 { + return nil, fmt.Errorf("no response choices returned") + } + + choice := openaiResp.Choices[0] + reasoning := choice.Message.ReasoningContent + if reasoning == "" { + reasoning = choice.Message.Reasoning + } + result := &ChatResponse{ + Content: choice.Message.Content, + ReasoningContent: reasoning, + Model: openaiResp.Model, + StopReason: choice.FinishReason, + InputTokens: openaiResp.Usage.PromptTokens, + OutputTokens: openaiResp.Usage.CompletionTokens, + } + if len(choice.Message.ToolCalls) > 0 { + result.StopReason = "tool_use" + for _, tc := range choice.Message.ToolCalls { + if strings.TrimSpace(tc.ID) == "" || strings.TrimSpace(tc.Function.Name) == "" { + return nil, fmt.Errorf("tool call is missing an id or function name") + } + input, ok := agentcapabilities.ParseProviderToolInput(tc.Function.Arguments) + if !ok { + return nil, fmt.Errorf("tool call %q returned invalid arguments", tc.Function.Name) + } + result.ToolCalls = append(result.ToolCalls, ToolCall{ + ID: tc.ID, + Name: tc.Function.Name, + Input: input, + }) + } + } + return result, nil +} + +func emitBufferedOpenAIResponse(callback StreamCallback, response *ChatResponse) error { + if response == nil { + return fmt.Errorf("empty non-streaming response") + } + if response.ReasoningContent != "" { + callback(StreamEvent{Type: "thinking", Data: ThinkingEvent{Text: response.ReasoningContent}}) + } + if response.Content != "" { + callback(StreamEvent{Type: "content", Data: ContentEvent{Text: response.Content}}) + } + for _, call := range response.ToolCalls { + callback(StreamEvent{ + Type: "tool_start", + Data: ToolStartEvent{ID: call.ID, Name: call.Name, Input: call.Input}.NormalizeCollections(), + }) + } + callback(StreamEvent{ + Type: "done", + Data: DoneEvent{ + StopReason: normalizeOpenAIStreamStopReason(response.StopReason, response.ToolCalls), + ToolCalls: response.ToolCalls, + InputTokens: response.InputTokens, + OutputTokens: response.OutputTokens, + }, + }) + return nil +} + +func openAIStreamingExplicitlyUnsupported(statusCode int, message string) bool { + if statusCode != http.StatusBadRequest && statusCode != http.StatusUnprocessableEntity && statusCode != http.StatusNotImplemented { + return false + } + lower := strings.ToLower(message) + return strings.Contains(lower, "streaming is not supported") || + strings.Contains(lower, "stream is not supported") || + strings.Contains(lower, "streaming unsupported") || + strings.Contains(lower, "stream must be false") || + strings.Contains(lower, "unsupported stream") +} + // isDeepSeek returns true if this client is configured for DeepSeek func (c *OpenAIClient) isDeepSeek() bool { return c.Name() == "deepseek" || strings.Contains(c.baseURL, "deepseek.com") @@ -351,6 +438,12 @@ func (c *OpenAIClient) applyProviderHeaders(req *http.Request) { req.Header.Set("X-Title", openrouterAppTitle) } +func (c *OpenAIClient) applyAuthorization(req *http.Request) { + if strings.TrimSpace(c.apiKey) != "" { + req.Header.Set("Authorization", "Bearer "+c.apiKey) + } +} + func (c *OpenAIClient) requestMaxTokens(req ChatRequest) int { if req.MaxTokens > 0 { return req.MaxTokens @@ -364,9 +457,21 @@ func (c *OpenAIClient) requestMaxTokens(req ChatRequest) int { // requiresMaxCompletionTokens returns true for models that need max_completion_tokens instead of max_tokens // Per OpenAI docs, o1/o3/o4 reasoning models require max_completion_tokens; max_tokens will error. func (c *OpenAIClient) requiresMaxCompletionTokens(model string) bool { + if c.isOpenRouter() { + return true + } + if !c.usesOfficialOpenAIEndpoint() { + return false + } return strings.HasPrefix(model, "o1") || strings.HasPrefix(model, "o3") || strings.HasPrefix(model, "o4") } +func (c *OpenAIClient) supportsStreamOptions() bool { + // stream_options is an OpenAI extension, not part of the minimum compatible + // protocol implemented by llama.cpp, LocalAI, and some LM Studio releases. + return c.Name() != "openai" || c.usesOfficialOpenAIEndpoint() +} + // convertToolChoiceToOpenAI converts our ToolChoice to OpenAI's format. // Pulse omits automatic tool_choice so tool use stays model-owned, and only // serializes native override modes when the caller explicitly requests them. @@ -466,13 +571,13 @@ func (c *OpenAIClient) Chat(ctx context.Context, req ChatRequest) (*ChatResponse Messages: messages, } - // Use max_completion_tokens for all OpenAI models (newer API, backward compatible) - // DeepSeek still uses max_tokens + // max_completion_tokens is required by official OpenAI reasoning models. + // The portable OpenAI-compatible field is max_tokens. if maxTokens := c.requestMaxTokens(req); maxTokens > 0 { - if c.isDeepSeek() { - openaiReq.MaxTokens = maxTokens - } else { + if c.requiresMaxCompletionTokens(model) { openaiReq.MaxCompletionTokens = maxTokens + } else { + openaiReq.MaxTokens = maxTokens } } @@ -551,7 +656,7 @@ func (c *OpenAIClient) Chat(ctx context.Context, req ChatRequest) (*ChatResponse } httpReq.Header.Set("Content-Type", "application/json") - httpReq.Header.Set("Authorization", "Bearer "+c.apiKey) + c.applyAuthorization(httpReq) c.applyProviderHeaders(httpReq) resp, err := c.client.Do(httpReq) @@ -612,41 +717,7 @@ func (c *OpenAIClient) Chat(ctx context.Context, req ChatRequest) (*ChatResponse return nil, fmt.Errorf("failed to parse response: %w", err) } - if len(openaiResp.Choices) == 0 { - return nil, fmt.Errorf("no response choices returned") - } - - choice := openaiResp.Choices[0] - - // Reasoning models expose chain-of-thought separately: DeepSeek's direct API - // in "reasoning_content", OpenRouter and other gateways in "reasoning". - reasoning := choice.Message.ReasoningContent - if reasoning == "" { - reasoning = choice.Message.Reasoning - } - - result := &ChatResponse{ - Content: choice.Message.Content, - ReasoningContent: reasoning, // surfaced as the turn's thinking - Model: openaiResp.Model, - StopReason: choice.FinishReason, - InputTokens: openaiResp.Usage.PromptTokens, - OutputTokens: openaiResp.Usage.CompletionTokens, - } - - // Convert tool calls from OpenAI format to our format - if len(choice.Message.ToolCalls) > 0 { - result.StopReason = "tool_use" // Normalize to match Anthropic's format - for _, tc := range choice.Message.ToolCalls { - result.ToolCalls = append(result.ToolCalls, ToolCall{ - ID: tc.ID, - Name: tc.Function.Name, - Input: agentcapabilities.ProviderToolInputOrRaw(tc.Function.Arguments), - }) - } - } - - return result, nil + return convertOpenAIResponse(openaiResp) } // TestConnection validates the API key by listing models @@ -669,7 +740,7 @@ func (c *OpenAIClient) testOpenRouterKey(ctx context.Context) error { return fmt.Errorf("failed to create request: %w", err) } - req.Header.Set("Authorization", "Bearer "+c.apiKey) + c.applyAuthorization(req) req.Header.Set("Accept", "application/json") c.applyProviderHeaders(req) @@ -870,16 +941,16 @@ func (c *OpenAIClient) ChatStream(ctx context.Context, req ChatRequest, callback Model: model, Messages: messages, Stream: true, - StreamOptions: &streamOptions{ - IncludeUsage: true, - }, + } + if c.supportsStreamOptions() { + openaiReq.StreamOptions = &streamOptions{IncludeUsage: true} } if maxTokens := c.requestMaxTokens(req); maxTokens > 0 { - if c.isDeepSeek() { - openaiReq.MaxTokens = maxTokens - } else { + if c.requiresMaxCompletionTokens(model) { openaiReq.MaxCompletionTokens = maxTokens + } else { + openaiReq.MaxTokens = maxTokens } } @@ -959,7 +1030,7 @@ func (c *OpenAIClient) ChatStream(ctx context.Context, req ChatRequest, callback } httpReq.Header.Set("Content-Type", "application/json") - httpReq.Header.Set("Authorization", "Bearer "+c.apiKey) + c.applyAuthorization(httpReq) httpReq.Header.Set("Accept", "text/event-stream") c.applyProviderHeaders(httpReq) @@ -989,6 +1060,16 @@ func (c *OpenAIClient) ChatStream(ctx context.Context, req ChatRequest, callback if isRetryableOpenAIStreamStatus(resp.StatusCode) { continue } + if openAIStreamingExplicitlyUnsupported(resp.StatusCode, errMsg) { + // Some otherwise compatible endpoints only implement buffered chat + // completions. Retry exactly once without stream=true, then emit the + // complete validated response through the canonical stream callback. + buffered, fallbackErr := c.Chat(ctx, req) + if fallbackErr != nil { + return fmt.Errorf("streaming unsupported and buffered fallback failed: %w", fallbackErr) + } + return emitBufferedOpenAIResponse(callback, buffered) + } return lastErr } @@ -997,6 +1078,21 @@ func (c *OpenAIClient) ChatStream(ctx context.Context, req ChatRequest, callback } defer resp.Body.Close() + if strings.Contains(strings.ToLower(resp.Header.Get("Content-Type")), "application/json") { + // A few compatible servers accept stream=true but return one ordinary + // JSON completion. Buffer and validate it before emitting any tool call, + // preserving fail-closed action semantics. + var bufferedResponse openaiResponse + if err := json.NewDecoder(resp.Body).Decode(&bufferedResponse); err != nil { + return fmt.Errorf("failed to parse buffered stream response: %w", err) + } + buffered, err := convertOpenAIResponse(bufferedResponse) + if err != nil { + return err + } + return emitBufferedOpenAIResponse(callback, buffered) + } + // Parse SSE stream reader := resp.Body buf := make([]byte, 4096) @@ -1208,7 +1304,7 @@ func (c *OpenAIClient) ListModels(ctx context.Context) ([]ModelInfo, error) { return nil, fmt.Errorf("failed to create request: %w", err) } - req.Header.Set("Authorization", "Bearer "+c.apiKey) + c.applyAuthorization(req) c.applyProviderHeaders(req) resp, err := c.client.Do(req) diff --git a/internal/ai/providers/openai_compatibility_test.go b/internal/ai/providers/openai_compatibility_test.go new file mode 100644 index 000000000..578911c44 --- /dev/null +++ b/internal/ai/providers/openai_compatibility_test.go @@ -0,0 +1,233 @@ +package providers + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// openAICompatibilityFixture models the minimum protocol shared by llama.cpp, +// LocalAI, LM Studio, and other OpenAI-compatible servers. Individual tests +// tighten or remove optional capabilities without changing the core fixture. +type openAICompatibilityFixture struct { + mu sync.Mutex + requests []map[string]interface{} + headers []http.Header +} + +func (f *openAICompatibilityFixture) capture(r *http.Request) (map[string]interface{}, error) { + var body map[string]interface{} + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + return nil, err + } + f.mu.Lock() + defer f.mu.Unlock() + f.requests = append(f.requests, body) + f.headers = append(f.headers, r.Header.Clone()) + return body, nil +} + +func TestOpenAICompatibleKeylessFixtureListsOpaqueModelsAndUsesPortableRequestShape(t *testing.T) { + fixture := &openAICompatibilityFixture{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Empty(t, r.Header.Get("Authorization")) + switch r.URL.Path { + case "/v1/models": + _ = json.NewEncoder(w).Encode(map[string]interface{}{ + "data": []map[string]interface{}{ + {"id": "HauhauCS/Qwen3.6-27B-Uncensored-HauhauCS-Balanced-Q5_K_P"}, + {"id": "local-model-without-known-prefix"}, + }, + }) + case "/v1/chat/completions": + body, err := fixture.capture(r) + require.NoError(t, err) + require.Equal(t, float64(128), body["max_tokens"]) + require.NotContains(t, body, "max_completion_tokens") + require.NotContains(t, body, "stream_options") + w.Header().Set("Content-Type", "text/event-stream") + _, _ = w.Write([]byte("data: {\"choices\":[{\"delta\":{\"content\":\"ok\"},\"finish_reason\":\"stop\"}]}\n\ndata: [DONE]\n\n")) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + client := NewOpenAICompatibleClient("openai", "", "local-model-without-known-prefix", server.URL, time.Second) + models, err := client.ListModels(context.Background()) + require.NoError(t, err) + require.Len(t, models, 2) + require.Equal(t, "HauhauCS/Qwen3.6-27B-Uncensored-HauhauCS-Balanced-Q5_K_P", models[0].ID) + require.Equal(t, "local-model-without-known-prefix", models[1].ID) + + var content string + err = client.ChatStream(context.Background(), ChatRequest{ + Messages: []Message{{Role: "user", Content: "hello"}}, + MaxTokens: 128, + }, func(event StreamEvent) { + if event.Type == "content" { + content += event.Data.(ContentEvent).Text + } + }) + require.NoError(t, err) + require.Equal(t, "ok", content) +} + +func TestOpenAICompatibleStreamingUnsupportedFallsBackToValidatedBufferedTools(t *testing.T) { + fixture := &openAICompatibilityFixture{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := fixture.capture(r) + require.NoError(t, err) + if body["stream"] == true { + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = w.Write([]byte(`{"error":{"message":"streaming is not supported; stream must be false"}}`)) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "model":"fixture-model", + "choices":[{ + "message":{ + "role":"assistant", + "reasoning_content":"checked schema", + "tool_calls":[{ + "id":"call-1", + "type":"function", + "function":{"name":"inspect_resource","arguments":"{\"resource_id\":\"vm-101\"}"} + }] + }, + "finish_reason":"tool_calls" + }], + "usage":{"prompt_tokens":5,"completion_tokens":7} + }`)) + })) + defer server.Close() + + client := NewOpenAICompatibleClient("openai", "", "fixture-model", server.URL, time.Second) + var eventTypes []string + var done DoneEvent + err := client.ChatStream(context.Background(), ChatRequest{ + Messages: []Message{{Role: "user", Content: "inspect"}}, + Tools: []Tool{{ + Name: "inspect_resource", + InputSchema: map[string]interface{}{"type": "object"}, + }}, + }, func(event StreamEvent) { + eventTypes = append(eventTypes, event.Type) + if event.Type == "done" { + done = event.Data.(DoneEvent) + } + }) + require.NoError(t, err) + require.Equal(t, []string{"thinking", "tool_start", "done"}, eventTypes) + require.Len(t, done.ToolCalls, 1) + require.Equal(t, "vm-101", done.ToolCalls[0].Input["resource_id"]) + require.Equal(t, 2, len(fixture.requests)) + require.Equal(t, true, fixture.requests[0]["stream"]) + require.NotContains(t, fixture.requests[1], "stream") +} + +func TestOpenAICompatibleServerMayReturnBufferedJSONForStreamRequest(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json; charset=utf-8") + _, _ = w.Write([]byte(`{ + "model":"fixture-model", + "choices":[{"message":{"role":"assistant","content":"buffered answer"},"finish_reason":"stop"}], + "usage":{"prompt_tokens":2,"completion_tokens":3} + }`)) + })) + defer server.Close() + + client := NewOpenAICompatibleClient("openai", "", "fixture-model", server.URL, time.Second) + var content string + var done bool + err := client.ChatStream(context.Background(), ChatRequest{ + Messages: []Message{{Role: "user", Content: "hello"}}, + }, func(event StreamEvent) { + switch event.Type { + case "content": + content += event.Data.(ContentEvent).Text + case "done": + done = true + } + }) + require.NoError(t, err) + require.Equal(t, "buffered answer", content) + require.True(t, done) +} + +func TestOpenAICompatibleMalformedBufferedResponseEmitsNothing(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"model":"fixture-model","choices":[]}`)) + })) + defer server.Close() + + client := NewOpenAICompatibleClient("openai", "", "fixture-model", server.URL, time.Second) + var eventCount int + err := client.ChatStream(context.Background(), ChatRequest{ + Messages: []Message{{Role: "user", Content: "hello"}}, + }, func(StreamEvent) { eventCount++ }) + require.Error(t, err) + require.Contains(t, err.Error(), "no response choices") + require.Zero(t, eventCount) +} + +func TestOpenAICompatibleRestrictedClientBlocksMetadataService(t *testing.T) { + client := NewOpenAICompatibleClient("openai", "", "fixture-model", "http://169.254.169.254/v1", 100*time.Millisecond) + _, err := client.ListModels(context.Background()) + require.Error(t, err) + require.True(t, + strings.Contains(err.Error(), "metadata service") || strings.Contains(err.Error(), "link-local"), + err.Error(), + ) +} + +func TestOpenAICompatibleConnectionKeepsTransportHealthSeparateFromSelectedModel(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"data":[{"id":"available-model"}]}`)) + })) + defer server.Close() + + client := NewOpenAICompatibleClient("openai", "", "missing-model", server.URL, time.Second) + err := client.TestConnection(context.Background()) + require.NoError(t, err) +} + +func TestOpenAICompatibleMalformedBufferedToolArgumentsEmitNothing(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "model":"fixture-model", + "choices":[{ + "message":{"tool_calls":[{ + "id":"call-1", + "type":"function", + "function":{"name":"inspect_resource","arguments":"{\"resource_id\":"} + }]}, + "finish_reason":"tool_calls" + }] + }`)) + })) + defer server.Close() + + client := NewOpenAICompatibleClient("openai", "", "fixture-model", server.URL, time.Second) + var eventCount int + err := client.ChatStream(context.Background(), ChatRequest{ + Messages: []Message{{Role: "user", Content: "inspect"}}, + Tools: []Tool{{ + Name: "inspect_resource", + InputSchema: map[string]interface{}{"type": "object"}, + }}, + }, func(StreamEvent) { eventCount++ }) + require.Error(t, err) + require.Contains(t, err.Error(), "invalid arguments") + require.Zero(t, eventCount) +} diff --git a/internal/ai/providers/openai_test.go b/internal/ai/providers/openai_test.go index b41def9cc..7ec04bcb5 100644 --- a/internal/ai/providers/openai_test.go +++ b/internal/ai/providers/openai_test.go @@ -187,16 +187,12 @@ func TestNewOpenAIClient_StreamTimeouts(t *testing.T) { // load and can spend minutes on prompt processing. Only the inter-chunk // gap keeps the short stall bound. client := NewOpenAIClient("sk-test", "gpt-4", "https://api.openai.com/v1", 0) - transport, ok := client.streamClient.Transport.(*http.Transport) - require.True(t, ok) - assert.Equal(t, 300*time.Second, transport.ResponseHeaderTimeout) + require.NotNil(t, client.streamClient.Transport) assert.Equal(t, openaiStreamChunkTimeout, client.streamChunkTimeout) assert.Equal(t, 300*time.Second, client.streamFirstChunkTimeout) shortTimeoutClient := NewOpenAIClient("sk-test", "gpt-4", "https://api.openai.com/v1", 2*time.Second) - shortTransport, ok := shortTimeoutClient.streamClient.Transport.(*http.Transport) - require.True(t, ok) - assert.Equal(t, 2*time.Second, shortTransport.ResponseHeaderTimeout) + require.NotNil(t, shortTimeoutClient.streamClient.Transport) assert.Equal(t, 2*time.Second, shortTimeoutClient.streamChunkTimeout) assert.Equal(t, 2*time.Second, shortTimeoutClient.streamFirstChunkTimeout) } @@ -1239,7 +1235,8 @@ func TestOpenAIClient_Chat_Success(t *testing.T) { var req openaiRequest require.NoError(t, json.NewDecoder(r.Body).Decode(&req)) assert.Equal(t, "gpt-4", req.Model) - assert.Equal(t, 123, req.MaxCompletionTokens) + assert.Equal(t, 123, req.MaxTokens) + assert.Zero(t, req.MaxCompletionTokens) assert.Equal(t, 0.7, req.Temperature) require.Len(t, req.Tools, 1) assert.Equal(t, "function", req.Tools[0].Type) diff --git a/internal/api/ai_handlers.go b/internal/api/ai_handlers.go index 1f05bb9b9..7e998eac8 100644 --- a/internal/api/ai_handlers.go +++ b/internal/api/ai_handlers.go @@ -36,6 +36,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/metrics" "github.com/rcourtman/pulse-go-rewrite/internal/mockmode" "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" "github.com/rcourtman/pulse-go-rewrite/internal/servicediscovery" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" "github.com/rcourtman/pulse-go-rewrite/internal/utils" @@ -1866,6 +1867,7 @@ func aiSettingsUpdateTouchesProviderConfig(req AISettingsUpdateRequest) bool { req.ClearOllamaURL != nil || req.ClearOllamaUsername != nil || req.ClearOllamaPassword != nil || + len(req.RemoveProviders) > 0 || req.CodexSubscriptionEnabled != nil || req.ClaudeSubscriptionEnabled != nil } @@ -2464,23 +2466,24 @@ type AISettingsResponse struct { // AIProviderDefinitionResponse exposes provider metadata without credentials. type AIProviderDefinitionResponse struct { - ID string `json:"id"` - DisplayName string `json:"display_name"` - Description string `json:"description"` - Protocol string `json:"protocol"` - DefaultModel string `json:"default_model,omitempty"` - DefaultBaseURL string `json:"default_base_url,omitempty"` - APIKeyField string `json:"api_key_field,omitempty"` - ConfiguredField string `json:"configured_field,omitempty"` - ClearKeyField string `json:"clear_key_field,omitempty"` - BaseURLField string `json:"base_url_field,omitempty"` - RequiresAPIKey bool `json:"requires_api_key"` - UserConfigurable bool `json:"user_configurable"` - Gateway bool `json:"gateway"` - Configured bool `json:"configured"` - ModelsDevProviderID string `json:"models_dev_provider_id,omitempty"` - EnvVars []string `json:"env_vars"` - DocsURL string `json:"docs_url,omitempty"` + ID string `json:"id"` + DisplayName string `json:"display_name"` + Description string `json:"description"` + Protocol string `json:"protocol"` + DefaultModel string `json:"default_model,omitempty"` + DefaultBaseURL string `json:"default_base_url,omitempty"` + APIKeyField string `json:"api_key_field,omitempty"` + ConfiguredField string `json:"configured_field,omitempty"` + ClearKeyField string `json:"clear_key_field,omitempty"` + BaseURLField string `json:"base_url_field,omitempty"` + RequiresAPIKey bool `json:"requires_api_key"` + APIKeyOptionalWithCustomBaseURL bool `json:"api_key_optional_with_custom_base_url,omitempty"` + UserConfigurable bool `json:"user_configurable"` + Gateway bool `json:"gateway"` + Configured bool `json:"configured"` + ModelsDevProviderID string `json:"models_dev_provider_id,omitempty"` + EnvVars []string `json:"env_vars"` + DocsURL string `json:"docs_url,omitempty"` // Patrol-blessed quickstart model for providers where users must pick a // model themselves (Ollama). Empty for curated-catalog providers. SuggestedModel string `json:"suggested_model,omitempty"` @@ -2544,25 +2547,26 @@ func aiProviderDefinitionResponses(settings *config.AIConfig) []AIProviderDefini continue } responses = append(responses, AIProviderDefinitionResponse{ - ID: def.ID, - DisplayName: def.DisplayName, - Description: def.Description, - Protocol: string(def.Protocol), - DefaultModel: config.DefaultModelForProvider(def.ID), - DefaultBaseURL: def.DefaultBaseURL, - APIKeyField: def.APIKeyField, - ConfiguredField: def.ConfiguredField, - ClearKeyField: def.ClearKeyField, - BaseURLField: def.BaseURLField, - RequiresAPIKey: def.RequiresAPIKey, - UserConfigurable: def.UserConfigurable, - Gateway: def.Gateway, - Configured: settings != nil && settings.HasProvider(def.ID), - ModelsDevProviderID: def.ModelsDevProviderID, - EnvVars: append([]string(nil), def.EnvVars...), - DocsURL: def.DocsURL, - SuggestedModel: def.SuggestedModel, - SuggestedModelNote: def.SuggestedModelNote, + ID: def.ID, + DisplayName: def.DisplayName, + Description: def.Description, + Protocol: string(def.Protocol), + DefaultModel: config.DefaultModelForProvider(def.ID), + DefaultBaseURL: def.DefaultBaseURL, + APIKeyField: def.APIKeyField, + ConfiguredField: def.ConfiguredField, + ClearKeyField: def.ClearKeyField, + BaseURLField: def.BaseURLField, + RequiresAPIKey: def.RequiresAPIKey, + APIKeyOptionalWithCustomBaseURL: def.APIKeyOptionalWithCustomBaseURL, + UserConfigurable: def.UserConfigurable, + Gateway: def.Gateway, + Configured: settings != nil && settings.HasProvider(def.ID), + ModelsDevProviderID: def.ModelsDevProviderID, + EnvVars: append([]string(nil), def.EnvVars...), + DocsURL: def.DocsURL, + SuggestedModel: def.SuggestedModel, + SuggestedModelNote: def.SuggestedModelNote, SuggestedModelEquivalents: append( []string(nil), def.SuggestedModelEquivalents...), }) @@ -2629,6 +2633,10 @@ type AISettingsUpdateRequest struct { ClearOllamaURL *bool `json:"clear_ollama_url,omitempty"` // Clear Ollama URL ClearOllamaUsername *bool `json:"clear_ollama_username,omitempty"` // Clear Ollama Basic Auth username ClearOllamaPassword *bool `json:"clear_ollama_password,omitempty"` // Clear Ollama Basic Auth password + // RemoveProviders deletes provider-owned credentials, endpoints, runtime + // options, and model selections. Legacy clear_* fields remain + // credential-specific for backwards compatibility. + RemoveProviders []string `json:"remove_providers,omitempty"` // Cost controls CostBudgetUSD30d *float64 `json:"cost_budget_usd_30d,omitempty"` // Request timeout (seconds) - for slow hardware running local models @@ -2938,7 +2946,17 @@ func (h *AISettingsHandler) HandleUpdateAISettings(w http.ResponseWriter, r *htt if req.ClearOllamaURL != nil && *req.ClearOllamaURL { settings.OllamaBaseURL = "" } else if req.OllamaBaseURL != nil { - settings.OllamaBaseURL = strings.TrimSpace(*req.OllamaBaseURL) + raw := strings.TrimSpace(*req.OllamaBaseURL) + if raw == "" { + settings.OllamaBaseURL = "" + } else { + normalized, err := securityutil.NormalizeHTTPBaseURL(raw, "") + if err != nil { + http.Error(w, "ollama_base_url "+err.Error(), http.StatusBadRequest) + return + } + settings.OllamaBaseURL = strings.TrimRight(normalized.String(), "/") + } } if req.ClearOllamaUsername != nil && *req.ClearOllamaUsername { settings.OllamaUsername = "" @@ -2959,7 +2977,17 @@ func (h *AISettingsHandler) HandleUpdateAISettings(w http.ResponseWriter, r *htt settings.OllamaKeepAlive = keepAlive } if req.OpenAIBaseURL != nil { - settings.OpenAIBaseURL = strings.TrimSpace(*req.OpenAIBaseURL) + raw := strings.TrimSpace(*req.OpenAIBaseURL) + if raw == "" { + settings.OpenAIBaseURL = "" + } else { + normalized, err := securityutil.NormalizeHTTPBaseURL(raw, "") + if err != nil { + http.Error(w, "openai_base_url "+err.Error(), http.StatusBadRequest) + return + } + settings.OpenAIBaseURL = strings.TrimRight(normalized.String(), "/") + } } if req.ZaiBaseURL != nil { settings.ZaiBaseURL = strings.TrimSpace(*req.ZaiBaseURL) @@ -2970,6 +2998,22 @@ func (h *AISettingsHandler) HandleUpdateAISettings(w http.ResponseWriter, r *htt if req.ClaudeSubscriptionEnabled != nil { settings.ClaudeSubscriptionEnabled = *req.ClaudeSubscriptionEnabled } + if len(req.RemoveProviders) > len(config.AIConfigurableProviderDefinitions()) { + http.Error(w, "too many providers requested for removal", http.StatusBadRequest) + return + } + removedProviders := make(map[string]struct{}, len(req.RemoveProviders)) + for _, providerName := range req.RemoveProviders { + providerName = strings.ToLower(strings.TrimSpace(providerName)) + if _, duplicate := removedProviders[providerName]; duplicate { + continue + } + if err := settings.RemoveProvider(providerName); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + removedProviders[providerName] = struct{}{} + } if req.Enabled != nil { // Only allow enabling if at least one BYOK/local provider is configured. @@ -3351,6 +3395,24 @@ func cachedPatrolModelReadinessSnapshot(aiService *ai.Service) *PatrolModelReadi return patrolModelReadinessSnapshot(result, recordedAt) } +func aiProviderOperationTimeout(cfg *config.AIConfig, providersToCheck ...string) time.Duration { + timeout := 30 * time.Second + if cfg == nil || cfg.GetRequestTimeout() <= timeout { + return timeout + } + for _, providerName := range providersToCheck { + switch strings.TrimSpace(providerName) { + case config.AIProviderOllama: + return cfg.GetRequestTimeout() + case config.AIProviderOpenAI: + if config.IsCustomOpenAICompatibleEndpoint(cfg.OpenAIBaseURL) { + return cfg.GetRequestTimeout() + } + } + } + return timeout +} + // HandleTestAIConnection tests the AI provider connection (POST /api/ai/test) // Auth is enforced by RequirePermission middleware at route registration; with // default authorizer, non-admin proxy users are hard-denied (with RBAC, deferred @@ -3367,7 +3429,12 @@ func (h *AISettingsHandler) HandleTestAIConnection(w http.ResponseWriter, r *htt return } - ctx, cancel := context.WithTimeout(r.Context(), 30*time.Second) + cfg := h.GetAIService(r.Context()).GetConfig() + selectedProvider := "" + if cfg != nil { + selectedProvider, _ = config.ParseModelString(cfg.GetModel()) + } + ctx, cancel := context.WithTimeout(r.Context(), aiProviderOperationTimeout(cfg, selectedProvider)) defer cancel() var testResult struct { @@ -3380,7 +3447,6 @@ func (h *AISettingsHandler) HandleTestAIConnection(w http.ResponseWriter, r *htt Action string `json:"action,omitempty"` } - cfg := h.GetAIService(r.Context()).GetConfig() err := h.GetAIService(r.Context()).TestConnection(ctx) if err != nil { diagnostic := ai.ClassifyProviderConnectionFailure(err) @@ -3502,9 +3568,6 @@ func (h *AISettingsHandler) HandleTestProvider(w http.ResponseWriter, r *http.Re } } - ctx, cancel := context.WithTimeout(r.Context(), 30*time.Second) - defer cancel() - testResult := newAIProviderTestResponse(provider) // Load config and create provider for testing @@ -3517,6 +3580,8 @@ func (h *AISettingsHandler) HandleTestProvider(w http.ResponseWriter, r *http.Re } return } + ctx, cancel := context.WithTimeout(r.Context(), aiProviderOperationTimeout(cfg, provider)) + defer cancel() // Check if provider is configured if !cfg.HasProvider(provider) { @@ -3587,7 +3652,12 @@ func (h *AISettingsHandler) HandleListModels(w http.ResponseWriter, r *http.Requ // Auth is enforced by RequireAuth + RequireScope middleware at the route level. - ctx, cancel := context.WithTimeout(r.Context(), 30*time.Second) + cfg := h.GetAIService(r.Context()).GetConfig() + configuredProviders := []string(nil) + if cfg != nil { + configuredProviders = cfg.GetConfiguredProviders() + } + ctx, cancel := context.WithTimeout(r.Context(), aiProviderOperationTimeout(cfg, configuredProviders...)) defer cancel() type ModelInfo struct { diff --git a/internal/api/ai_handlers_test.go b/internal/api/ai_handlers_test.go index 710d7eb83..0773a55dd 100644 --- a/internal/api/ai_handlers_test.go +++ b/internal/api/ai_handlers_test.go @@ -849,6 +849,89 @@ func TestAISettingsHandler_UpdateSettings_OllamaKeepAlive(t *testing.T) { require.Equal(t, "24h", saved.OllamaKeepAlive) } +func TestAISettingsHandler_RemoveProviderPersistsCompleteLifecycle(t *testing.T) { + t.Parallel() + + tmp := t.TempDir() + appConfig := &config.Config{DataPath: tmp} + persistence := config.NewConfigPersistence(tmp) + aiConfig := config.NewDefaultAIConfig() + aiConfig.Enabled = true + aiConfig.Model = "openai:opaque-local-model" + aiConfig.ChatModel = "openai:opaque-local-model" + aiConfig.PatrolModel = "openai:opaque-local-model" + aiConfig.OpenAIAPIKey = "secret" + aiConfig.OpenAIBaseURL = "http://127.0.0.1:8080/v1" + require.NoError(t, persistence.SaveAIConfig(*aiConfig)) + + handler := newTestAISettingsHandler(appConfig, persistence, nil) + body, err := json.Marshal(AISettingsUpdateRequest{ + RemoveProviders: []string{config.AIProviderOpenAI}, + }) + require.NoError(t, err) + req := newLoopbackRequest(http.MethodPut, "/api/settings/ai/update", bytes.NewReader(body)) + rec := httptest.NewRecorder() + handler.HandleUpdateAISettings(rec, req) + + require.Equal(t, http.StatusOK, rec.Code, rec.Body.String()) + var response AISettingsResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &response)) + require.False(t, response.Enabled) + require.False(t, response.OpenAIConfigured) + require.Empty(t, response.Model) + require.Empty(t, response.ChatModel) + require.Empty(t, response.PatrolModel) + require.Empty(t, response.ConfiguredProviders) + + reopened, err := config.NewConfigPersistence(tmp).LoadAIConfig() + require.NoError(t, err) + require.Empty(t, reopened.OpenAIAPIKey) + require.Empty(t, reopened.OpenAIBaseURL) + require.Empty(t, reopened.Model) + require.Empty(t, reopened.ChatModel) + require.Empty(t, reopened.PatrolModel) + require.False(t, reopened.Enabled) +} + +func TestAISettingsHandler_KeylessOpenAICompatibleSetupIsConfiguredAndValidated(t *testing.T) { + t.Parallel() + + tmp := t.TempDir() + appConfig := &config.Config{DataPath: tmp} + persistence := config.NewConfigPersistence(tmp) + handler := newTestAISettingsHandler(appConfig, persistence, nil) + + body, err := json.Marshal(AISettingsUpdateRequest{ + OpenAIBaseURL: ptr("http://127.0.0.1:8080/v1/"), + }) + require.NoError(t, err) + req := newLoopbackRequest(http.MethodPut, "/api/settings/ai/update", bytes.NewReader(body)) + rec := httptest.NewRecorder() + handler.HandleUpdateAISettings(rec, req) + + require.Equal(t, http.StatusOK, rec.Code, rec.Body.String()) + var response AISettingsResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &response)) + require.True(t, response.OpenAIConfigured) + require.Contains(t, response.ConfiguredProviders, config.AIProviderOpenAI) + require.Equal(t, "http://127.0.0.1:8080/v1", response.OpenAIBaseURL) + + reopened, err := config.NewConfigPersistence(tmp).LoadAIConfig() + require.NoError(t, err) + require.Empty(t, reopened.OpenAIAPIKey) + require.Equal(t, "http://127.0.0.1:8080/v1", reopened.OpenAIBaseURL) + + body, err = json.Marshal(AISettingsUpdateRequest{ + OpenAIBaseURL: ptr("http://127.0.0.1:8080/v1?token=secret"), + }) + require.NoError(t, err) + req = newLoopbackRequest(http.MethodPut, "/api/settings/ai/update", bytes.NewReader(body)) + rec = httptest.NewRecorder() + handler.HandleUpdateAISettings(rec, req) + require.Equal(t, http.StatusBadRequest, rec.Code) + require.Contains(t, rec.Body.String(), "openai_base_url") +} + func TestAISettingsHandler_UpdateSettingsRejectsInvalidOllamaKeepAlive(t *testing.T) { t.Parallel() diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index f8859d1fa..bc4149801 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -2041,7 +2041,7 @@ func TestContract_AISettingsUpdateProviderResolutionJSONSnapshot(t *testing.T) { "ollama_configured":true, "ollama_base_url":%q, "ollama_password_set":false, - "ollama_keep_alive":"30s", + "ollama_keep_alive":"", "configured_providers":["ollama"], "control_level":"read_only", "protected_guests":[], @@ -2236,7 +2236,7 @@ func TestContract_AISettingsBYOKOverrideDoesNotExposeQuickstartInventoryJSONSnap "ollama_configured":false, "ollama_base_url":"http://localhost:11434", "ollama_password_set":false, - "ollama_keep_alive":"30s", + "ollama_keep_alive":"", "configured_providers":["openai"], "control_level":"read_only", "protected_guests":[], @@ -4490,7 +4490,7 @@ func TestContract_HostedAISettingsDoesNotAutoBootstrapQuickstartJSONSnapshot(t * "ollama_configured":false, "ollama_base_url":"http://localhost:11434", "ollama_password_set":false, - "ollama_keep_alive":"30s", + "ollama_keep_alive":"", "configured_providers":[], "control_level":"read_only", "protected_guests":[], @@ -4561,7 +4561,7 @@ func TestContract_AISettingsRetiredQuickstartAliasJSONSnapshot(t *testing.T) { "ollama_configured":false, "ollama_base_url":"http://localhost:11434", "ollama_password_set":false, - "ollama_keep_alive":"30s", + "ollama_keep_alive":"", "configured_providers":[], "control_level":"read_only", "protected_guests":[], @@ -4637,7 +4637,7 @@ func TestContract_AISettingsOllamaAuthJSONSnapshot(t *testing.T) { "ollama_base_url":"http://ollama.example:11434", "ollama_username":"unai", "ollama_password_set":true, - "ollama_keep_alive":"30s", + "ollama_keep_alive":"", "configured_providers":["ollama"], "control_level":"read_only", "protected_guests":[], @@ -5206,7 +5206,7 @@ func TestContract_HostedTenantAISettingsDoesNotAutoBootstrapQuickstartJSONSnapsh "ollama_configured":false, "ollama_base_url":"http://localhost:11434", "ollama_password_set":false, - "ollama_keep_alive":"30s", + "ollama_keep_alive":"", "configured_providers":[], "control_level":"read_only", "protected_guests":[], diff --git a/internal/config/ai.go b/internal/config/ai.go index 5788472b5..29e7ce724 100644 --- a/internal/config/ai.go +++ b/internal/config/ai.go @@ -3,6 +3,7 @@ package config import ( "fmt" "math" + "net/url" "strconv" "strings" "time" @@ -189,7 +190,10 @@ const ( // Pulse-hosted model aliases from pre-GA config. DefaultAIModelQuickstart = "pulse-hosted" DefaultOllamaBaseURL = "http://localhost:11434" - DefaultOllamaKeepAlive = "30s" + // DefaultOllamaKeepAlive is intentionally empty. Omitting keep_alive lets + // each Ollama server apply its own operator-configured default. Pulse only + // overrides that policy when the operator saves an explicit value. + DefaultOllamaKeepAlive = "" DefaultOpenRouterBaseURL = "https://openrouter.ai/api/v1" DefaultDeepSeekBaseURL = "https://api.deepseek.com" DefaultGeminiBaseURL = "https://generativelanguage.googleapis.com/v1beta" @@ -244,8 +248,7 @@ func NewDefaultAIConfig() *AIConfig { Enabled: false, Model: "", AuthMethod: AuthMethodAPIKey, - // Pulse keeps the v6 cost-control default explicit. Operators can - // clear this value to let the Ollama server's own default apply. + // Empty inherits the Ollama server's keep_alive policy. OllamaKeepAlive: DefaultOllamaKeepAlive, // Patrol defaults - enabled when AI is enabled // Default to 6 hour intervals (much more token-efficient than 15 min) @@ -394,9 +397,9 @@ func NormalizeOllamaKeepAlive(value string) (string, error) { return "", fmt.Errorf("must be a duration such as 30s, 5m, or 24h; seconds such as 3600; -1 to keep loaded; 0 to unload; or empty to use the Ollama server default") } -// GetOllamaKeepAlive returns the configured keep_alive value. A nil config -// keeps the Pulse default; an explicitly empty config value is preserved so -// callers can omit keep_alive and defer to the Ollama server. +// GetOllamaKeepAlive returns the configured keep_alive value. Empty, including +// the default for a nil config, means callers omit keep_alive and defer to the +// Ollama server. func (c *AIConfig) GetOllamaKeepAlive() string { if c == nil { return DefaultOllamaKeepAlive @@ -422,7 +425,10 @@ func (c *AIConfig) HasProvider(provider string) bool { case AIProviderAnthropic: return c.AnthropicAPIKey != "" case AIProviderOpenAI: - return c.OpenAIAPIKey != "" + // A custom OpenAI-compatible endpoint may be intentionally keyless + // (for example llama.cpp, LocalAI, or LM Studio). The official OpenAI + // endpoint still requires a key. + return strings.TrimSpace(c.OpenAIAPIKey) != "" || IsCustomOpenAICompatibleEndpoint(c.OpenAIBaseURL) case AIProviderOpenRouter: return c.OpenRouterAPIKey != "" case AIProviderDeepSeek: @@ -453,6 +459,108 @@ func (c *AIConfig) HasProvider(provider string) bool { } } +// ProviderRequiresAPIKey reports whether the selected provider route requires +// an API key. OpenAI-compatible custom endpoints may be keyless; every hosted +// provider route keeps the registry's credential requirement. +func (c *AIConfig) ProviderRequiresAPIKey(provider string) bool { + provider = strings.ToLower(strings.TrimSpace(provider)) + def, ok := LookupAIProviderDefinition(provider) + if !ok || !def.RequiresAPIKey { + return false + } + if provider == AIProviderOpenAI && c != nil && IsCustomOpenAICompatibleEndpoint(c.OpenAIBaseURL) { + return false + } + return true +} + +// IsCustomOpenAICompatibleEndpoint distinguishes an operator-supplied +// compatible server from an explicitly saved official OpenAI URL. Saving the +// official host must never turn OpenAI into a keyless provider. +func IsCustomOpenAICompatibleEndpoint(raw string) bool { + raw = strings.TrimSpace(raw) + if raw == "" { + return false + } + parsed, err := url.Parse(raw) + if err != nil || parsed.Hostname() == "" { + return false + } + host := strings.TrimSuffix(parsed.Hostname(), ".") + return !strings.EqualFold(host, "api.openai.com") +} + +// RemoveProvider deletes all provider-owned configuration and any model +// selections routed through that provider. It is deliberately distinct from +// the legacy clear-key fields, which only rotate one credential. +func (c *AIConfig) RemoveProvider(provider string) error { + if c == nil { + return fmt.Errorf("Pulse Assistant config is nil") + } + provider = strings.ToLower(strings.TrimSpace(provider)) + def, ok := LookupAIProviderDefinition(provider) + if !ok || !def.UserConfigurable { + return fmt.Errorf("unknown provider %q", provider) + } + + switch provider { + case AIProviderAnthropic: + c.AnthropicAPIKey = "" + c.ClearOAuthTokens() + case AIProviderOpenAI: + c.OpenAIAPIKey = "" + c.OpenAIBaseURL = "" + case AIProviderOpenRouter: + c.OpenRouterAPIKey = "" + case AIProviderDeepSeek: + c.DeepSeekAPIKey = "" + case AIProviderGemini: + c.GeminiAPIKey = "" + case AIProviderZai: + c.ZaiAPIKey = "" + c.ZaiBaseURL = "" + case AIProviderGroq: + c.GroqAPIKey = "" + case AIProviderMistral: + c.MistralAPIKey = "" + case AIProviderCerebras: + c.CerebrasAPIKey = "" + case AIProviderTogether: + c.TogetherAPIKey = "" + case AIProviderFireworks: + c.FireworksAPIKey = "" + case AIProviderOllama: + c.OllamaBaseURL = "" + c.OllamaUsername = "" + c.OllamaPassword = "" + c.OllamaKeepAlive = DefaultOllamaKeepAlive + case AIProviderCodexSubscription: + c.CodexSubscriptionEnabled = false + case AIProviderClaudeSubscription: + c.ClaudeSubscriptionEnabled = false + } + + clearModel := func(model *string) { + if strings.TrimSpace(*model) == "" { + return + } + modelProvider, _ := ParseModelString(*model) + if modelProvider == provider { + *model = "" + } + } + clearModel(&c.Model) + clearModel(&c.ChatModel) + clearModel(&c.PatrolModel) + clearModel(&c.DiscoveryModel) + clearModel(&c.AutoFixModel) + + if len(c.GetConfiguredProviders()) == 0 { + c.Enabled = false + } + return nil +} + // GetConfiguredProviders returns a list of all providers with credentials configured func (c *AIConfig) GetConfiguredProviders() []string { if c == nil { diff --git a/internal/config/ai_config_test.go b/internal/config/ai_config_test.go index f3ef99dd7..b8ac2aaa5 100644 --- a/internal/config/ai_config_test.go +++ b/internal/config/ai_config_test.go @@ -435,6 +435,18 @@ func TestAIProviderDefinitions_CanonicalDirectProviderRegistry(t *testing.T) { t.Fatalf("%s must declare API settings fields: %#v", provider, def) } } + openAI, ok := LookupAIProviderDefinition(AIProviderOpenAI) + if !ok { + t.Fatal("missing OpenAI provider definition") + } + if !openAI.RequiresAPIKey || !openAI.APIKeyOptionalWithCustomBaseURL || openAI.BaseURLField != "openai_base_url" { + t.Fatalf("OpenAI custom-endpoint auth contract is incomplete: %#v", openAI) + } + for _, def := range defs { + if def.ID != AIProviderOpenAI && def.APIKeyOptionalWithCustomBaseURL { + t.Fatalf("%s must not inherit OpenAI custom-endpoint keyless auth", def.ID) + } + } if _, ok := LookupAIProviderDefinition(AIProviderQuickstart); !ok { t.Fatalf("retired quickstart marker should remain known for migration cleanup") diff --git a/internal/config/ai_provider_lifecycle_test.go b/internal/config/ai_provider_lifecycle_test.go new file mode 100644 index 000000000..ef8aec4f6 --- /dev/null +++ b/internal/config/ai_provider_lifecycle_test.go @@ -0,0 +1,105 @@ +package config + +import ( + "os" + "testing" +) + +func TestOpenAICustomEndpointMayBeConfiguredWithoutKey(t *testing.T) { + cfg := &AIConfig{OpenAIBaseURL: "http://127.0.0.1:8080/v1"} + if !cfg.HasProvider(AIProviderOpenAI) { + t.Fatal("custom OpenAI-compatible endpoint should configure the provider without a key") + } + if cfg.ProviderRequiresAPIKey(AIProviderOpenAI) { + t.Fatal("custom OpenAI-compatible endpoint should not require a key") + } + + cfg.OpenAIBaseURL = "" + if cfg.HasProvider(AIProviderOpenAI) { + t.Fatal("official OpenAI route without a key must not be configured") + } + if !cfg.ProviderRequiresAPIKey(AIProviderOpenAI) { + t.Fatal("official OpenAI route must require a key") + } + cfg.OpenAIBaseURL = "https://api.openai.com/v1" + if cfg.HasProvider(AIProviderOpenAI) || !cfg.ProviderRequiresAPIKey(AIProviderOpenAI) { + t.Fatal("explicit official OpenAI URL must remain key-required") + } + cfg.OpenAIBaseURL = "https://api.openai.com./v1" + if cfg.HasProvider(AIProviderOpenAI) || !cfg.ProviderRequiresAPIKey(AIProviderOpenAI) { + t.Fatal("DNS-qualified official OpenAI URL must remain key-required") + } +} + +func TestRemoveProviderClearsOwnedSecretsEndpointOptionsAndModels(t *testing.T) { + cfg := &AIConfig{ + Enabled: true, + Model: "openai:opaque-model", + ChatModel: "openai:chat-model", + PatrolModel: "ollama:qwen3:8b", + DiscoveryModel: "openai:discovery-model", + AutoFixModel: "openai:fix-model", + OpenAIAPIKey: "secret", + OpenAIBaseURL: "http://127.0.0.1:8080/v1", + OllamaBaseURL: "http://127.0.0.1:11434", + OllamaUsername: "operator", + OllamaPassword: "password", + OllamaKeepAlive: "24h", + } + + if err := cfg.RemoveProvider(AIProviderOpenAI); err != nil { + t.Fatalf("RemoveProvider(openai) error = %v", err) + } + if cfg.OpenAIAPIKey != "" || cfg.OpenAIBaseURL != "" { + t.Fatal("OpenAI removal left provider-owned secrets or endpoint behind") + } + if cfg.Model != "" || cfg.ChatModel != "" || cfg.DiscoveryModel != "" || cfg.AutoFixModel != "" { + t.Fatalf("OpenAI model selections were not cleared: %+v", cfg) + } + if cfg.PatrolModel != "ollama:qwen3:8b" || !cfg.Enabled { + t.Fatal("unrelated Ollama selection/provider should remain enabled") + } + + if err := cfg.RemoveProvider(AIProviderOllama); err != nil { + t.Fatalf("RemoveProvider(ollama) error = %v", err) + } + if cfg.OllamaBaseURL != "" || cfg.OllamaUsername != "" || cfg.OllamaPassword != "" || cfg.OllamaKeepAlive != "" { + t.Fatal("Ollama removal left provider-owned state behind") + } + if cfg.PatrolModel != "" || cfg.Enabled { + t.Fatal("removing the final provider must clear its model and disable AI") + } +} + +func TestOllamaKeepAliveInheritsByDefaultAndPersistsExplicitValues(t *testing.T) { + dir := t.TempDir() + persistence := NewConfigPersistence(dir) + + defaults := NewDefaultAIConfig() + if defaults.GetOllamaKeepAlive() != "" { + t.Fatalf("default keep_alive = %q, want server inheritance", defaults.GetOllamaKeepAlive()) + } + if err := persistence.SaveAIConfig(AIConfig{OllamaBaseURL: "http://127.0.0.1:11434", OllamaKeepAlive: "24h"}); err != nil { + t.Fatalf("SaveAIConfig() error = %v", err) + } + reloaded, err := NewConfigPersistence(dir).LoadAIConfig() + if err != nil { + t.Fatalf("LoadAIConfig() error = %v", err) + } + if reloaded.GetOllamaKeepAlive() != "24h" { + t.Fatalf("reloaded keep_alive = %q, want 24h", reloaded.GetOllamaKeepAlive()) + } + + legacyDir := t.TempDir() + legacy := NewConfigPersistence(legacyDir) + if err := os.WriteFile(legacy.aiFile, []byte(`{"enabled":false,"ollama_base_url":"http://127.0.0.1:11434"}`), 0o600); err != nil { + t.Fatalf("write legacy config: %v", err) + } + legacyReloaded, err := legacy.LoadAIConfig() + if err != nil { + t.Fatalf("LoadAIConfig(legacy) error = %v", err) + } + if legacyReloaded.GetOllamaKeepAlive() != "" { + t.Fatalf("legacy missing keep_alive = %q, want server inheritance", legacyReloaded.GetOllamaKeepAlive()) + } +} diff --git a/internal/config/ai_providers.go b/internal/config/ai_providers.go index b4197185d..ffe39f1e9 100644 --- a/internal/config/ai_providers.go +++ b/internal/config/ai_providers.go @@ -27,23 +27,27 @@ type AIProviderModelDefinition struct { // AIProviderDefinition is the canonical internal registry record for provider // metadata and runtime capability selection. type AIProviderDefinition struct { - ID string - DisplayName string - Description string - Protocol AIProviderProtocol - DefaultModel string - DefaultBaseURL string - APIKeyField string - ConfiguredField string - ClearKeyField string - BaseURLField string - RequiresAPIKey bool - UserConfigurable bool - Gateway bool - ModelsDevProviderID string - EnvVars []string - DocsURL string - FallbackModels []AIProviderModelDefinition + ID string + DisplayName string + Description string + Protocol AIProviderProtocol + DefaultModel string + DefaultBaseURL string + APIKeyField string + ConfiguredField string + ClearKeyField string + BaseURLField string + RequiresAPIKey bool + // APIKeyOptionalWithCustomBaseURL marks providers whose official route + // requires a key but whose operator-supplied compatible endpoint may be + // intentionally keyless. + APIKeyOptionalWithCustomBaseURL bool + UserConfigurable bool + Gateway bool + ModelsDevProviderID string + EnvVars []string + DocsURL string + FallbackModels []AIProviderModelDefinition // SuggestedModel is a model verified to pass Patrol's tool-call // preflight, surfaced as a guided quickstart on the provider's setup // row. Only set for providers where users must choose a model without @@ -85,19 +89,20 @@ func aiProviderDefinitions() []AIProviderDefinition { DocsURL: "https://docs.anthropic.com/en/api/getting-started", }, { - ID: AIProviderOpenAI, - DisplayName: "OpenAI", - Description: "GPT and reasoning models from OpenAI, or a custom OpenAI-compatible endpoint", - Protocol: AIProviderProtocolOpenAICompatible, - DefaultModel: "gpt-4o", - APIKeyField: "openai_api_key", - ConfiguredField: "openai_configured", - ClearKeyField: "clear_openai_key", - BaseURLField: "openai_base_url", - RequiresAPIKey: true, - UserConfigurable: true, - EnvVars: []string{"OPENAI_API_KEY"}, - DocsURL: "https://platform.openai.com/docs/api-reference/chat", + ID: AIProviderOpenAI, + DisplayName: "OpenAI", + Description: "GPT and reasoning models from OpenAI, or a custom OpenAI-compatible endpoint", + Protocol: AIProviderProtocolOpenAICompatible, + DefaultModel: "gpt-4o", + APIKeyField: "openai_api_key", + ConfiguredField: "openai_configured", + ClearKeyField: "clear_openai_key", + BaseURLField: "openai_base_url", + RequiresAPIKey: true, + APIKeyOptionalWithCustomBaseURL: true, + UserConfigurable: true, + EnvVars: []string{"OPENAI_API_KEY"}, + DocsURL: "https://platform.openai.com/docs/api-reference/chat", }, { ID: AIProviderOpenRouter, diff --git a/pkg/securityutil/outbound_http.go b/pkg/securityutil/outbound_http.go index 9000d206b..02a0741a8 100644 --- a/pkg/securityutil/outbound_http.go +++ b/pkg/securityutil/outbound_http.go @@ -18,8 +18,11 @@ type RestrictedOutboundHTTPOptions struct { AllowedSchemes []string AllowPrivateIPs bool AllowLoopback bool - TLSConfig *tls.Config - ResolveIPAddrs func(ctx context.Context, host string) ([]net.IPAddr, error) + // ResponseHeaderTimeout bounds the wait for response headers without + // imposing an overall deadline on a streaming response body. + ResponseHeaderTimeout time.Duration + TLSConfig *tls.Config + ResolveIPAddrs func(ctx context.Context, host string) ([]net.IPAddr, error) } var resolveOutboundFetchIPs = net.DefaultResolver.LookupIPAddr @@ -202,7 +205,7 @@ func sameOriginRedirectPolicy(opts RestrictedOutboundHTTPOptions) func(req *http } } -func cloneRestrictedTransport(tlsConfig *tls.Config) *http.Transport { +func cloneRestrictedTransport(opts RestrictedOutboundHTTPOptions) *http.Transport { transport, ok := http.DefaultTransport.(*http.Transport) var clone *http.Transport if ok && transport != nil { @@ -212,8 +215,8 @@ func cloneRestrictedTransport(tlsConfig *tls.Config) *http.Transport { } switch { - case tlsConfig != nil: - clone.TLSClientConfig = tlsConfig.Clone() + case opts.TLSConfig != nil: + clone.TLSClientConfig = opts.TLSConfig.Clone() case clone.TLSClientConfig != nil: clone.TLSClientConfig = clone.TLSClientConfig.Clone() default: @@ -223,6 +226,9 @@ func cloneRestrictedTransport(tlsConfig *tls.Config) *http.Transport { if clone.TLSClientConfig.MinVersion < tls.VersionTLS12 { clone.TLSClientConfig.MinVersion = tls.VersionTLS12 } + if opts.ResponseHeaderTimeout > 0 { + clone.ResponseHeaderTimeout = opts.ResponseHeaderTimeout + } return clone } @@ -252,7 +258,7 @@ func (r *restrictedRoundTripper) RoundTrip(req *http.Request) (*http.Response, e // NewRestrictedOutboundHTTPClient returns an HTTP client that validates redirects and pins direct outbound dials // to the permitted resolved IPs for the requested host, trying each in resolution order until one connects. func NewRestrictedOutboundHTTPClient(timeout time.Duration, opts RestrictedOutboundHTTPOptions) *http.Client { - transport := cloneRestrictedTransport(opts.TLSConfig) + transport := cloneRestrictedTransport(opts) transport.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { host, port, err := net.SplitHostPort(addr) if err != nil { diff --git a/scripts/release_control/ai_runtime_docs_policy_test.py b/scripts/release_control/ai_runtime_docs_policy_test.py index 733a84917..e5f7e9797 100644 --- a/scripts/release_control/ai_runtime_docs_policy_test.py +++ b/scripts/release_control/ai_runtime_docs_policy_test.py @@ -237,6 +237,23 @@ class AIRuntimeDocsPolicyTest(unittest.TestCase): self.assertIn("`patrol_readiness` usage category", normalized_content) self.assertNotIn("Patrol model ready", content) + def test_public_ai_overview_defines_local_compatible_provider_lifecycle(self) -> None: + content = read_repo_text("docs/AI.md") + normalized_content = " ".join(content.split()) + + self.assertIn("llama.cpp, LocalAI, LM Studio", normalized_content) + self.assertIn( + "API key is optional when the custom endpoint is intentionally keyless", + normalized_content, + ) + self.assertIn("Custom OpenAI-compatible model catalogs are authoritative", normalized_content) + self.assertIn("Pulse lists every non-empty model ID returned by the endpoint", normalized_content) + self.assertIn("Pulse omits empty Authorization headers", normalized_content) + self.assertIn("Blank is the default", normalized_content) + self.assertIn("the Ollama server's own policy applies", normalized_content) + self.assertIn("Provider transport health is reported independently from Patrol capability", normalized_content) + self.assertIn("does not weaken Patrol's fail-closed tool/action admission", normalized_content) + def test_public_ai_docs_use_current_surface_naming(self) -> None: for doc_path in PUBLIC_AI_DOC_PATHS: with self.subTest(doc_path=doc_path): diff --git a/tests/integration/tests/52-ai-settings-provider-setup.spec.ts b/tests/integration/tests/52-ai-settings-provider-setup.spec.ts index 674c142c5..4b33363bf 100644 --- a/tests/integration/tests/52-ai-settings-provider-setup.spec.ts +++ b/tests/integration/tests/52-ai-settings-provider-setup.spec.ts @@ -29,6 +29,23 @@ type MockAISettings = { model?: string; checks: Array>; }; + patrol_model_readiness?: { + probe_version: string; + success: boolean; + transport_healthy: boolean; + patrol_capable: boolean; + status: string; + provider: string; + model: string; + duration_ms: number; + cause: string; + summary: string; + recommendation: string; + dimensions: Record>; + modes: Record>; + recorded_at: string; + recorded_at_unix: number; + }; }; const baseSettings = (): MockAISettings => ({ @@ -139,7 +156,9 @@ test.describe("Pulse Intelligence settings provider setup", () => { // First enable routes through the Set up Pulse Intelligence dialog: // pick a provider, submit the key, and let the backend select models. // No model may be hardcoded into the update payload. - await page.getByRole("button", { name: "Enable Pulse Intelligence" }).click(); + await page + .getByRole("button", { name: "Enable Pulse Intelligence" }) + .click(); const setupDialog = page.getByRole("dialog", { name: "Set up Pulse Intelligence", }); @@ -163,9 +182,10 @@ test.describe("Pulse Intelligence settings provider setup", () => { // Per-model overrides moved into the section panels; the panel signals // enabled state and the backend-selected shared default model here. - await expect( - page.getByLabel("Enable Pulse Intelligence"), - ).toHaveAttribute("aria-pressed", "true"); + await expect(page.getByLabel("Enable Pulse Intelligence")).toHaveAttribute( + "aria-pressed", + "true", + ); }); test("provider setup surfaces saved Patrol readiness warnings with provider and model context", async ({ @@ -268,15 +288,147 @@ test.describe("Pulse Intelligence settings provider setup", () => { // The readiness warning carries the provider and model context from the // saved patrol_readiness payload. - await expect( - page.getByText(/but Patrol is not ready/), - ).toBeVisible(); + await expect(page.getByText(/but Patrol is not ready/)).toBeVisible(); await expect(page.getByText("Provider: OpenRouter")).toBeVisible(); await expect( page.getByText("Model: openrouter:deepseek/deepseek-r1"), ).toBeVisible(); }); + test("keyless custom endpoint keeps provider health separate from Patrol tool capability", async ({ + page, + }, testInfo) => { + test.skip( + testInfo.project.name.startsWith("mobile-"), + "Desktop-only settings coverage", + ); + await ensureAuthenticated(page); + + const settings: MockAISettings = { + ...baseSettings(), + enabled: true, + configured: true, + model: "openai:model-without-known-prefix", + patrol_model: "openai:model-without-known-prefix", + openai_configured: true, + configured_providers: ["openai"], + patrol_model_readiness: { + probe_version: "patrol-readiness/v1", + success: false, + transport_healthy: true, + patrol_capable: false, + status: "warning", + provider: "openai", + model: "model-without-known-prefix", + duration_ms: 60000, + cause: "model_tool_support_unverified", + summary: + "The provider is healthy for ordinary chat, but the selected model did not demonstrate Patrol's streaming tool protocol.", + recommendation: + "Keep using this route for ordinary Assistant chat, or choose a Patrol model with reliable streaming tool use.", + dimensions: { + connectivity: { + status: "pass", + summary: "Provider and selected model are reachable.", + }, + tool_protocol: { + status: "fail", + summary: "Exact tool protocol passed 0/3 scenarios.", + attempts: 3, + passed: 0, + }, + context_quality: { + status: "fail", + summary: "Patrol-shaped context fixtures passed 0/2 scenarios.", + attempts: 2, + passed: 0, + }, + latency: { + status: "pass", + summary: + "Warm median 22.5s; projected 8-turn Watch-only loop 3m0s.", + }, + }, + modes: { + monitor: { + status: "not_suitable", + summary: + "The model did not pass the minimum Watch-only protocol and context checks.", + }, + approval: { + status: "not_suitable", + summary: "Ask first requires all Watch-only checks to pass first.", + }, + assisted: { + status: "not_assessed", + summary: + "Requires an extended remediation and verification canary.", + }, + full: { + status: "not_assessed", + summary: "Requires an extended governed Autopilot canary.", + }, + }, + recorded_at: "2026-07-24T10:00:00Z", + recorded_at_unix: 1784887200, + }, + }; + + await page.route("**/api/settings/ai", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify(settings), + }); + }); + await page.route("**/api/ai/models", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + models: [ + { + id: "openai:model-without-known-prefix", + name: "Opaque local model", + provider: "openai", + }, + ], + }), + }); + }); + await page.route("**/api/ai/test/openai", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + success: true, + message: "Connection successful", + provider: "openai", + model: "openai:model-without-known-prefix", + }), + }); + }); + await page.route("**/api/ai/chat/sessions", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify([]), + }); + }); + + await page.goto("/settings/pulse-intelligence/patrol", { + waitUntil: "domcontentloaded", + }); + await expect( + page.getByText("Provider connected; Patrol capability not verified"), + ).toBeVisible(); + await expect( + page.getByText("Provider and selected model are reachable."), + ).toBeVisible(); + await expect(page.getByText("0/3 · Failed")).toBeVisible(); + await expect(page.getByText("Opaque local model").first()).toBeVisible(); + }); + test("settings save failure keeps provider preflight recommendation context", async ({ page, }, testInfo) => {