fix(ai-providers): let a keyless server be configured, and say what the model list and an http base are doing - #3043
Merged
Conversation
…he model list and an http base are doing
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The four defects the #3040 investigation found in the AI provider settings and left for a follow-up (#3042 shipped the five the endpoint fix could not be verified without). Each was verified against the code before it was planned, and none depends on the others.
1. A Custom provider could not be saved against a server that wants no key
AIProviderType.authStylenamed Cursor, xAI, OpenCode Zen and the local providers explicitly and let.customfall through todefault: .apiKey. Save stayed dim, Test Connection stayed disabled and the model list never loaded, so a self-hosted vLLM, an llama-server behind a reverse proxy, or LM Studio with API-key checking off could not be configured at all, whiledocs/features/ai-assistant.mdxadvertises Custom as taking "any OpenAI-compatible endpoint"..customnow returns.optionalApiKey. Nothing else needed changing: of the eleven placesauthStyleis read, seven already bucket.apiKeywith.optionalApiKey, and the four that compare== .apiKeyexactly are the four gates this is meant to open (isSaveEnabled, Test Connection'sdisabled, and the early returns infetchModelsandtestProvider).authStyleis computed and never persisted, so no stored configuration migrates.AISettingsView.customStatusTextalready fell back to showing the host when no key was stored, which reads as though it was written expecting this.2. The model list never reloaded when the API key changed
The Base URL field's
onChangescheduled a refetch; all three API-key fields only cleared the test result. Typing the endpoint first and pasting the key second, which is the natural order, left the Model picker holding only Other… with no spinner, no error and no Reload, and the only way out was to go back and retype a character in the Base URL.All three key fields now schedule the refetch. The Cursor and xAI fields matter as much as the generic one: both are
.optionalApiKey, so the empty-key guard never fired for them and their key change was simply lost.The guard itself moved out of the view into
AIModelListFetchGate, which is pure and therefore testable, and the case that blocks the fetch now says so (Enter an API key to load this provider's models.) instead of clearing the error and rendering nothing.3. A malformed model list read as "this server has no models"
fetchOpenAIModelsandfetchOllamaModelseach ran one combined guard over the JSON parse and the array cast and returned[]when either failed, so a gateway answering 200 with the wrong shape was indistinguishable from a server that genuinely serves no models. Each is now two guards: a parse failure and a missing array both report, and only a well-formed array, empty or not, returns.This covers the six types that route through
OpenAICompatibleProvider(OpenRouter, OpenCode Zen, Ollama, llama.cpp, MLX, Custom). Claude and Gemini keep their curated offline list, which the six have no equivalent of;OpenAIResponsesProvideralready threw.OllamaDetectorhas the same shape and is deliberately left alone, since it is a silent launch probe with no error surface.4. No caution when an API key goes over http to another machine
App Transport Security is
NSAllowsArbitraryLoadsapp-wide, the Base URL field takes anyhttp(s)URL, and nothing distinguished the two. The field now carries an inline caution when the resolved base is plaintext to a host that is not this machine.Warn, do not refuse.
MCPServerConfiguration.validaterefuses a non-loopback http endpoint outright, and that is right for MCP, which grants remote tool execution. A chat request carrying the user's own key to a server they chose is theirs to decide, and LiteLLM, vLLM and Ollama on a LAN box are ordinary setups. ATS itself is untouched; changing it app-wide is a far larger change than this.The wording follows what is actually sent: a key that will travel gets
Your API key is sent unencrypted over http to this host., and a keyless Custom server or a remote Ollama getsRequests to this host are sent unencrypted over http.The loopback predicate
Five copies of "is this host loopback" already exist and they disagree: one counts
0.0.0.0andlocalhost.localdomain, another matches four exact spellings and misses the rest of127.0.0.0/8. Rather than adding a sixth, the most complete one moves out ofExternalConnectionTrustKeyintoLoopbackHost, and both callers use it.ExternalConnectionTrustStoreTests' twelve cases are the proof that the extraction changed nothing. The other three copies keep their own semantics and are left alone; consolidating them is a separate change.Verification
verify.sh buildverify.sh test(16 suites)verify.sh test(10 suites, after review fixes)verify.sh lint(12 files)verify.sh docsSuites run:
AIModelListFetchGateTests,LoopbackHostTests,CustomProviderRegistrationTests,AIEndpointTests,AIProviderModelFetchTests,OpenAICompatibleProviderConnectionTests,OpenAICompatibleProviderParserTests,OpenAICompatibleProviderEncodingTests,LocalProviderRegistrationTests,ExternalConnectionTrustStoreTests,AIProviderCapabilitiesTests,AIProviderFactoryResolveTests,AIProviderFactoryCacheTests,AIProviderErrorTests,XAIRegistrationTests,CursorRegistrationTests.xAI is unaffected throughout: both its paths build
OpenAIResponsesProviderorXAIGrokProvider, neverOpenAICompatibleProvider.No UI automation. The reachable assertions here are a field label and two caption lines in the provider detail sheet; the behaviour worth pinning is the fetch gate and the loopback predicate, and both are covered as pure functions instead.
Review
Codex reviewed the working tree and raised four, all fixed in this branch:
.onAppearfetch was no longer short-circuited, so a newly added Custom provider, whose Base URL starts empty, asked a transport with no URL for a model list and drewInvalid endpoint:before the user had typed anything. The gate gained a.missingEndpointcase ahead of the key check.isFetchingModels, and the blocked branch returned without clearing it either. Every blocked branch now resets it.Not in this PR
OpenAICompatibleProvider.testConnection's.ollamabranch is unreachable: its sole caller is the Test Connection button, which lives insideauthSection, and Ollama'sauthStyleis.none, so that section rendersEmptyView(). Ollama, llama.cpp, MLX and Claude Agent have no Test Connection affordance at all. Giving the.none-auth providers one is a UI change with its own design questions, so it is reported rather than bundled here.