feat: Custom inference endpoints for third-party API models#10781
feat: Custom inference endpoints for third-party API models#10781dagmfactory wants to merge 7 commits into
Conversation
…s info-tooltip max-width
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds local storage, settings UI, model-selector integration, and request wiring for custom OpenAI-compatible inference endpoints.
Concerns
- Custom endpoint secrets can be sent even when the workspace BYOK entitlement is disabled.
- The settings UI now enables BYOK fields/custom endpoint setup based only on AI availability and the feature flag, not BYOK eligibility.
- Custom endpoint URLs are accepted with only a non-empty check before being persisted and sent to the agent backend.
- Custom model IDs still resolve when the feature flag is disabled, while request wiring omits the provider registry.
- Secure-storage writes for API keys were changed to fire-and-forget spawned callbacks, which can persist stale endpoint/API-key state out of order.
Security
- The request path bypasses the BYOK entitlement by calling
custom_model_providers_for_request(true). - Arbitrary custom endpoint URLs need HTTPS/public-host validation before they can become cloud inference targets.
Verdict
Found: 0 critical, 5 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| ); | ||
| let custom_model_providers = FeatureFlag::CustomInferenceEndpoints | ||
| .is_enabled() | ||
| .then(|| api_key_manager.custom_model_providers_for_request(true)) |
There was a problem hiding this comment.
true bypasses the workspace BYOK entitlement computed above, so custom endpoint API keys are sent even when BYOK is disabled. Use is_byo_enabled for this request path.
There was a problem hiding this comment.
do we mean to hardcode true as the argument here? or should this be disabled when the user is an enterprise user?
| let ai_settings = AISettings::as_ref(app); | ||
| let is_any_ai_enabled = ai_settings.is_any_ai_enabled(app); | ||
| let is_byo_enabled = UserWorkspaces::as_ref(app).is_byo_api_key_enabled(app); | ||
| let is_enabled = is_any_ai_enabled; |
There was a problem hiding this comment.
is_byo_enabled check, so users without BYOK can edit provider keys and configure custom endpoints despite the upgrade CTA. Keep the input/add-button enablement tied to both is_any_ai_enabled and BYOK eligibility.
There was a problem hiding this comment.
This seems valid? We want to make sure we don't regress any disabled state.
| .is_empty() | ||
| }); | ||
| !name.trim().is_empty() | ||
| && !url.trim().is_empty() |
There was a problem hiding this comment.
| /// Resolves an `LLMId` against the user's custom-endpoint LLMs. | ||
| /// Returns `None` if the id isn't a known custom model `config_key`. | ||
| pub fn custom_llm_info_for_id(&self, id: &LLMId) -> Option<&LLMInfo> { | ||
| self.custom_llms.iter().find(|info| info.id == *id) |
There was a problem hiding this comment.
CustomInferenceEndpoints is disabled, while request wiring omits custom_model_providers in that state. Gate this helper too or reconcile stored custom selections back to a built-in model when the flag is off.
| // already emitted, so the UI updates immediately while the | ||
| // potentially slow platform secure-storage call runs in a | ||
| // subsequent main-thread callback. | ||
| ctx.spawn(async move { json }, |_, json, ctx| { |
There was a problem hiding this comment.
There was a problem hiding this comment.
Did this change get made back when we were investigating lag? If there wasn't an issue with the previous writing mechanism I'm hesitant to make changes.
danielpeng2
left a comment
There was a problem hiding this comment.
Doing some testing, going to leave comments about behaviour as they come up:
- The "Add endpoint" button isn't enabled after I fill things in, might be another ctx.notify issue: https://www.loom.com/share/5f945cf47641482ca4d0033b11c9d615
| ); | ||
| let custom_model_providers = FeatureFlag::CustomInferenceEndpoints | ||
| .is_enabled() | ||
| .then(|| api_key_manager.custom_model_providers_for_request(true)) |
There was a problem hiding this comment.
do we mean to hardcode true as the argument here? or should this be disabled when the user is an enterprise user?
| /// Returns `None` when BYOK is disabled or no endpoint has both a non-empty URL and API key. | ||
| pub fn custom_model_providers_for_request( | ||
| &self, | ||
| include_byo_keys: bool, |
There was a problem hiding this comment.
is include_byo_keys the right name for this condition, naming it something like include_custom_model_providers seems more clear? alternatively, we could lift this check outside of the method and check it before calling this method, which might be a simpler API.
| |me, _event: &ApiKeyManagerEvent, ctx| { | ||
| me.rebuild_custom_llms(ctx); | ||
| me.reconcile_disabled_model_preferences(ctx); | ||
| ctx.emit(LLMPreferencesEvent::UpdatedAvailableLLMs); |
There was a problem hiding this comment.
Can we make it so that we emit the LLMPreferencesEvent::UpdatedAvailableLLMs event directly at the site where the custom endpoints are modified? I'm a little worried about this indirection.
| self.get_computer_use_available() | ||
| .choices | ||
| .iter() | ||
| .chain(self.custom_llm_choices()) |
There was a problem hiding this comment.
can we not allow custom models in computer use for now? this requires some special model setup that we aren't doing
| }, | ||
| description: Some(format!("Custom · {}", endpoint.name)), | ||
| disable_reason: None, | ||
| vision_supported: false, |
There was a problem hiding this comment.
I believe this field controls whether the user can input images when the model is selected. I think it'd be good to leave this on by default and let it error if the model doesn't actually support vision.
| let ai_settings = AISettings::as_ref(app); | ||
| let is_any_ai_enabled = ai_settings.is_any_ai_enabled(app); | ||
| let is_byo_enabled = UserWorkspaces::as_ref(app).is_byo_api_key_enabled(app); | ||
| let is_enabled = is_any_ai_enabled; |
There was a problem hiding this comment.
This seems valid? We want to make sure we don't regress any disabled state.
| } | ||
| } | ||
|
|
||
| /// Creates a new menu item with vertically stacked primary and secondary text. |
There was a problem hiding this comment.
nit: this comment got truncated
| // Lazy migration: stamp a UUID `config_key` on any pre-existing custom model that | ||
| // was persisted before this field landed. If any were filled, schedule a re-save so | ||
| // we don't repeat the work on the next launch. | ||
| let migrated = Self::backfill_missing_config_keys(&mut keys); | ||
| if migrated { | ||
| let json = match serde_json::to_string(&keys) { | ||
| Ok(json) => json, | ||
| Err(e) => { | ||
| log::error!("Failed to re-serialize API keys after migration: {e:#}"); | ||
| return keys; | ||
| } | ||
| }; | ||
| ctx.spawn(async move { json }, |_, json, ctx| { | ||
| if let Err(e) = ctx.secure_storage().write_value(SECURE_STORAGE_KEY, &json) { | ||
| log::error!("Failed to write migrated API keys to secure storage: {e:#}"); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Do we need this migration logic? Users can't add custom models right now, so I don't see how they would create one without a UUID.
| // already emitted, so the UI updates immediately while the | ||
| // potentially slow platform secure-storage call runs in a | ||
| // subsequent main-thread callback. | ||
| ctx.spawn(async move { json }, |_, json, ctx| { |
There was a problem hiding this comment.
Did this change get made back when we were investigating lag? If there wasn't an issue with the previous writing mechanism I'm hesitant to make changes.
danielpeng2
left a comment
There was a problem hiding this comment.
Nice job on this! Approving to unblock.
My most important comments are the logic/behavoural ones. We don't have to fix all the UI nits in this PR. Important thing is that we get this in before the bug bash tomorrow.




Description
Adds support for custom inference endpoints in AI settings, allowing users to add, edit, and remove third-party OpenAI-compatible API endpoints. Custom endpoint models surface in all model selectors when the feature flag is enabled.
Changes
crates/ai/src/api_keys.rs,app/src/ai/llms.rs) — stores endpoint name, URL, API key, and model list with aliases.crates/warp_features/src/lib.rs) — gates the feature behindCustomInferenceEndpoints.app/src/ai/agent/api.rs,app/src/ai/agent/api/impl_tests.rs) — passescustom_model_providersto the agent API.app/src/settings_view/ai_page.rs) — new "Custom inference" section with endpoint cards, add/edit/remove flow.app/src/settings_view/custom_inference_modal.rs) — form for entering endpoint details and model names/aliases.app/src/settings_view/remove_custom_endpoint_confirmation_dialog.rs) — confirms destructive removal with model chips preview.Screenshots / Videos
https://www.loom.com/share/0a8b6ee1ce0d43e09b698e621be8416d
Linked Issue
https://linear.app/warpdotdev/issue/QUALITY-605/bring-your-own-endpoint
Remaining work:
Disable bring your own feature for enterprise accounts
Order custom models right after auto models in the model picker
Warp TOS hyperlink in custom inference info icon
Testing
./script/runCustomInferenceEndpointsserialization/deserialization incrates/ai/src/api_keys_tests.rsandapp/src/ai/llms_tests.rs.impl_tests.rscoverage for agent API wiring.Agent Mode
CHANGELOG-NEW-FEATURE: Add custom inference endpoints to AI settings, allowing users to bring their own OpenAI-compatible third-party API endpoints and use them across all model selectors.