feat: add complete Azure OpenAI provider support#523
Merged
jamiepine merged 12 commits intospacedriveapp:mainfrom Apr 2, 2026
Merged
feat: add complete Azure OpenAI provider support#523jamiepine merged 12 commits intospacedriveapp:mainfrom
jamiepine merged 12 commits intospacedriveapp:mainfrom
Conversation
Full implementation of Azure OpenAI provider with UI configuration, validation, and documentation. ## Features Added - Azure provider type with dedicated configuration UI in Settings - Support for Azure-specific fields: base_url, api_version, and deployment name - Validation for Azure endpoints (must end with .openai.azure.com) - API version format validation (YYYY-MM-DD or YYYY-MM-DD-preview) - Deployment name validation supporting dots (e.g., gpt-5.2) - Automatic model routing for Azure deployments ## Backend Changes - New handler with comprehensive validation - Modified to handle Azure provider - Modified to route Azure requests correctly - Added Azure to provider status detection and config management - Support for Azure in LLM model initialization and routing ## Frontend Changes - Azure provider entry in PROVIDERS array - State management for Azure-specific fields - Azure-specific validation in test and save handlers - Auto-population of Azure config when editing - Updated API client to pass Azure fields correctly ## Documentation - Added Azure configuration examples to config.mdx - Updated routing.mdx with Azure model format - Added Azure to README.md with complete configuration guide - Updated legacy keys table with missing providers ## Technical Details - Azure uses custom provider pattern (not legacy keys) - Model format: azure/<deployment-name> - Base URL must contain '.openai.azure.com' - Deployment names support alphanumeric, hyphens, and dots Files changed: - src/api/providers.rs: Azure handlers and validation - src/api/server.rs: Route registration - src/config/types.rs: Azure config types - src/config/providers.rs: Azure provider defaults - src/llm/model.rs: Azure model initialization - interface/src/routes/Settings.tsx: Azure UI components - interface/src/api/client.ts: Azure API calls - docs/content/docs/(configuration)/config.mdx: Azure docs - docs/content/docs/(core)/routing.mdx: Azure routing - README.md: Azure configuration example
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds first-class Azure OpenAI support across docs, config/schema, backend provider endpoints and validation/persistence, runtime Azure request/endpoint/auth handling, and frontend UI/client flows for configuring/testing Azure provider settings. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
src/config/load.rs (1)
463-476: Consolidate duplicated provider bootstrap logic inload_from_env.The provider insertion logic is duplicated (e.g., OpenRouter appears in both blocks), which increases drift risk whenever
ProviderConfigfields evolve. Consider extracting one shared insertion path/helper and calling it once.Also applies to: 581-594
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config/load.rs` around lines 463 - 476, The insertion logic for provider bootstrapping (e.g., the block using llm.openrouter_key to insert into llm.providers with ProviderConfig fields) is duplicated; extract a single helper function (e.g., new_provider_entry or insert_provider_if_key) called from load_from_env that accepts provider id ("openrouter"), api_type, base_url, api_key, name, use_bearer_auth, extra_headers, api_version, deployment and performs the llm.providers.entry(...).or_insert_with(...) creation so both the OpenRouter block and the other duplicated block (also around the later 581-594 region) call this helper instead of repeating the ProviderConfig literal; update callers to pass openrouter_key and openrouter_extra_headers() (and the equivalent values for the other provider) so ProviderConfig instantiation is centralized.src/llm/model.rs (1)
1012-1032: Add an explicit arm forApiType::KiloGatewayinstead of relying on the_catch-all.The match statement at lines 1012–1032 currently routes
ApiType::KiloGatewaythrough the generic error handler via_. Adding an explicit arm for this variant—either with its own error message or by routing it to the same endpoint path if it's compatible—ensures that futureApiTypeadditions fail at compile time rather than regressing to a runtime error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/llm/model.rs` around lines 1012 - 1032, The match on provider_config.api_type that sets endpoint_path should include an explicit ApiType::KiloGateway arm rather than falling to the _ case; update the match in the block that assigns endpoint_path (currently handling ApiType::OpenAiCompletions, OpenAiResponses, OpenAiChatCompletions, Gemini, Azure, Anthropic, and _) to add ApiType::KiloGateway and either return the appropriate "/v1/chat/completions" path if KiloGateway is OpenAI-compatible or return a clear CompletionError::ProviderError message (similar style to the Azure/Anthropic arms) if it is not compatible, so future ApiType additions fail at compile time instead of routing through the generic `_` case.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@interface/src/routes/Settings.tsx`:
- Around line 414-415: The client-side Azure base URL check in Settings.tsx is
too permissive and doesn't normalize trailing slashes; update all checks that
currently use azureBaseUrl.trim().includes(".openai.azure.com") (and the
corresponding testResult branches that call setTestResult(...)) to first trim
and remove a trailing '/' (e.g., let normalized =
azureBaseUrl.trim().replace(/\/+$/, '')) and then validate with
normalized.endsWith(".openai.azure.com"); apply this same change to the other
occurrences referenced so the client-side validation matches the server-side
logic.
- Around line 423-429: The Azure deployment changes are not being used when
testing or saving: derive a new azureModel = `azure/${azureDeployment.trim()}`
and replace usages of modelInput for Azure flows so the actual requests and
saves use the derived model; specifically pass azureModel into
testModelMutation.mutateAsync (where provider === "azure"), into the update/save
mutation call (the update mutation around the later mutateAsync), and include
deployment/baseUrl/apiVersion/azureModel in the currentSignature calculation (so
changes to azureDeployment, azureBaseUrl, or azureApiVersion invalidate tests).
Locate and update the references to testModelMutation.mutateAsync, the
update/save mutateAsync call, and the currentSignature variable to use
azureModel and include the extra Azure fields.
- Around line 730-758: The Azure prefill request in the onEdit handler can write
stale data into the shared dialog state; update the
api.getProviderConfig("azure") call to be guarded or cancelable: create an
AbortController (or capture a local snapshot of editingProvider) before calling
api.getProviderConfig and either pass the signal to the request or, when the
promise resolves, check that editingProvider === "azure" (and the dialog is
still open) before calling setAzureBaseUrl, setAzureApiVersion,
setAzureDeployment, setModelInput, or setKeyInput; also ensure any previously
started controller is aborted when starting a new edit to prevent out-of-order
responses from mutating the form.
In `@README.md`:
- Around line 201-203: Remove the unsupported shorthand key in the README
example: delete the `[llm] azure_api_key = "env:AZURE_API_KEY"` entry and
replace/document the correct config path ` [llm.provider.azure].api_key` (or
show only the canonical `[llm.provider.azure].api_key = "env:AZURE_API_KEY"`
example) so the README matches the parsed config schema and avoids misleading
users; update the example around the existing `[llm]` block to reference
`api_key` under `llm.provider.azure` instead of the removed shorthand.
In `@src/api/providers.rs`:
- Around line 1073-1085: The ProviderConfigResponse struct currently includes
api_key which leaks stored credentials to the client; remove the api_key field
from ProviderConfigResponse (and any other response types at the other location
noted around lines 1143-1157) so responses only return metadata (base_url,
api_version, deployment, success, message). Update the handler code that
builds/serializes ProviderConfigResponse (look for the constructor or serializer
usage for ProviderConfigResponse and the other response struct at the second
location) to no longer read or emit the stored api_key, and instead keep
server-side logic in the provider update/create methods to preserve the existing
secret when the incoming request omits an api_key (i.e., do not overwrite stored
credentials with an empty/absent value). Ensure client-side Settings.tsx stops
expecting a prefilled key from this response.
- Around line 979-988: The code currently validates Azure routing by inspecting
normalized_model; instead, change the Azure provider checks and default-routing
seeding to use the client-supplied deployment value as the source of truth:
validate that deployment (not model) matches the Azure provider (e.g., call
model_matches_provider("azure", normalized_deployment) or add an equivalent
deployment_matches_provider check), seed any missing default routes from the
normalized/deployment string (use deployment when building routes and
default_agent routing), and update the branches around the
ProviderUpdateResponse return and the related block at lines ~1019-1048 to
ensure the saved deployment and applied routes come from deployment so they
cannot diverge from the persisted deployment value (update references to
normalized_model -> normalized_deployment or deployment where appropriate).
In `@src/llm/model.rs`:
- Around line 163-208: The code slices base_url (variable) assuming a leading
"https://" and later reconstructs the endpoint, which breaks for inputs like
"http://..."; update the Azure-path logic to parse and validate the origin once
(e.g., using Url::parse on provider_config.base_url), assert the scheme is
"https" and the host ends_with ".openai.azure.com", then derive a canonical
origin (scheme + "://" + host) and reuse that origin when building the endpoint
string instead of manually trimming/adding "https://"; change the
resource/deployment/api_version usage to format!("{}
/openai/deployments/{}/chat/completions?api-version={}", origin, deployment,
api_version) so both validation and endpoint construction use the same parsed
origin.
---
Nitpick comments:
In `@src/config/load.rs`:
- Around line 463-476: The insertion logic for provider bootstrapping (e.g., the
block using llm.openrouter_key to insert into llm.providers with ProviderConfig
fields) is duplicated; extract a single helper function (e.g.,
new_provider_entry or insert_provider_if_key) called from load_from_env that
accepts provider id ("openrouter"), api_type, base_url, api_key, name,
use_bearer_auth, extra_headers, api_version, deployment and performs the
llm.providers.entry(...).or_insert_with(...) creation so both the OpenRouter
block and the other duplicated block (also around the later 581-594 region) call
this helper instead of repeating the ProviderConfig literal; update callers to
pass openrouter_key and openrouter_extra_headers() (and the equivalent values
for the other provider) so ProviderConfig instantiation is centralized.
In `@src/llm/model.rs`:
- Around line 1012-1032: The match on provider_config.api_type that sets
endpoint_path should include an explicit ApiType::KiloGateway arm rather than
falling to the _ case; update the match in the block that assigns endpoint_path
(currently handling ApiType::OpenAiCompletions, OpenAiResponses,
OpenAiChatCompletions, Gemini, Azure, Anthropic, and _) to add
ApiType::KiloGateway and either return the appropriate "/v1/chat/completions"
path if KiloGateway is OpenAI-compatible or return a clear
CompletionError::ProviderError message (similar style to the Azure/Anthropic
arms) if it is not compatible, so future ApiType additions fail at compile time
instead of routing through the generic `_` case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3174b2ad-fd8c-4ffb-8d05-4d3e0898336c
📒 Files selected for processing (14)
README.mddocs/content/docs/(configuration)/config.mdxdocs/content/docs/(core)/routing.mdxinterface/src/api/client.tsinterface/src/lib/providerIcons.tsxinterface/src/routes/Settings.tsxsrc/api/providers.rssrc/api/server.rssrc/config/load.rssrc/config/providers.rssrc/config/toml_schema.rssrc/config/types.rssrc/llm/manager.rssrc/llm/model.rs
Comment on lines
423
to
+429
| const result = await testModelMutation.mutateAsync({ | ||
| provider: editingProvider, | ||
| apiKey: keyInput.trim(), | ||
| model: modelInput.trim(), | ||
| baseUrl: editingProvider === "azure" ? azureBaseUrl.trim() : undefined, | ||
| apiVersion: editingProvider === "azure" ? azureApiVersion.trim() : undefined, | ||
| deployment: editingProvider === "azure" ? azureDeployment.trim() : undefined, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f 'Settings.tsx' interface/src/routes/Repository: spacedriveapp/spacebot
Length of output: 99
🏁 Script executed:
wc -l interface/src/routes/Settings.tsxRepository: spacedriveapp/spacebot
Length of output: 104
🏁 Script executed:
sed -n '420,435p' interface/src/routes/Settings.tsx | cat -nRepository: spacedriveapp/spacebot
Length of output: 812
🏁 Script executed:
sed -n '470,485p' interface/src/routes/Settings.tsx | cat -nRepository: spacedriveapp/spacebot
Length of output: 583
🏁 Script executed:
sed -n '920,935p' interface/src/routes/Settings.tsx | cat -nRepository: spacedriveapp/spacebot
Length of output: 627
🏁 Script executed:
# Search for azureDeployment, azureModel, and modelInput usage
rg -n 'azureDeployment|azureModel|modelInput' interface/src/routes/Settings.tsx | head -50Repository: spacedriveapp/spacebot
Length of output: 1263
🏁 Script executed:
sed -n '390,420p' interface/src/routes/Settings.tsx | cat -nRepository: spacedriveapp/spacebot
Length of output: 1432
🏁 Script executed:
sed -n '395,415p' interface/src/routes/Settings.tsx | cat -nRepository: spacedriveapp/spacebot
Length of output: 1139
🏁 Script executed:
# Check what the disabled state condition is for the test button
sed -n '960,970p' interface/src/routes/Settings.tsx | cat -nRepository: spacedriveapp/spacebot
Length of output: 632
🏁 Script executed:
# Look for where modelInput is being used and if there's Azure-specific model handling elsewhere
rg -B 3 -A 3 'azureModel' interface/src/routes/Settings.tsxRepository: spacedriveapp/spacebot
Length of output: 48
🏁 Script executed:
sed -n '285,330p' interface/src/routes/Settings.tsx | cat -nRepository: spacedriveapp/spacebot
Length of output: 2121
🏁 Script executed:
# Check the saveMutation and how currentSignature affects the flow
sed -n '440,465p' interface/src/routes/Settings.tsx | cat -nRepository: spacedriveapp/spacebot
Length of output: 1093
🏁 Script executed:
# Look at the testedSignature state and how it's being used
rg -n 'testedSignature' interface/src/routes/Settings.tsxRepository: spacedriveapp/spacebot
Length of output: 273
🏁 Script executed:
# Check if there's any code that auto-updates modelInput based on azureDeployment
rg -B 5 -A 5 'setModelInput' interface/src/routes/Settings.tsx | head -80Repository: spacedriveapp/spacebot
Length of output: 2457
🏁 Script executed:
# Check what happens when provider changes to Azure or back
sed -n '1050,1150p' interface/src/routes/Settings.tsx | cat -nRepository: spacedriveapp/spacebot
Length of output: 4163
Derive the Azure model from azureDeployment for test and save operations.
The disabled field displays azure/${azureDeployment}, but both test and update mutations still send modelInput, which retains its previously loaded value. When a user changes the deployment, the displayed model updates but the actual request uses the outdated model string, causing the test/save to operate on the wrong configuration. Additionally, currentSignature only includes provider, keyInput, and modelInput—it omits deployment, baseUrl, and apiVersion—so changing only these Azure fields won't invalidate the tested signature, leaving the UI showing outdated test results.
Compute azureModel from azureDeployment.trim() with the format azure/${deployment}, and use it consistently in:
- Test mutation (line 426)
- Update mutation (line 477)
- Current signature (line 393)
This ensures test results are invalidated when any Azure configuration changes and requests use the correct derived model.
🛠️ Suggested change
+ const azureModel = azureDeployment.trim()
+ ? `azure/${azureDeployment.trim()}`
+ : "";
- const currentSignature = `${editingProvider ?? ""}|${keyInput.trim()}|${modelInput.trim()}`;
+ const currentModel = editingProvider === "azure" ? azureModel : modelInput.trim();
+ const currentSignature = `${editingProvider ?? ""}|${keyInput.trim()}|${currentModel}|${azureBaseUrl.trim()}|${azureApiVersion.trim()}`;
...
- model: modelInput.trim(),
+ model: editingProvider === "azure" ? azureModel : modelInput.trim(),
...
- value={`azure/${azureDeployment || "deployment"}`}
+ value={azureModel || "azure/deployment"}Also applies to: 473-480, 924-929
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@interface/src/routes/Settings.tsx` around lines 423 - 429, The Azure
deployment changes are not being used when testing or saving: derive a new
azureModel = `azure/${azureDeployment.trim()}` and replace usages of modelInput
for Azure flows so the actual requests and saves use the derived model;
specifically pass azureModel into testModelMutation.mutateAsync (where provider
=== "azure"), into the update/save mutation call (the update mutation around the
later mutateAsync), and include deployment/baseUrl/apiVersion/azureModel in the
currentSignature calculation (so changes to azureDeployment, azureBaseUrl, or
azureApiVersion invalidate tests). Locate and update the references to
testModelMutation.mutateAsync, the update/save mutateAsync call, and the
currentSignature variable to use azureModel and include the extra Azure fields.
Comment on lines
+163
to
+208
| let base_url = provider_config.base_url.trim_end_matches('/'); | ||
|
|
||
| // Validate that the endpoint is actually an Azure OpenAI endpoint | ||
| if !base_url.ends_with(".openai.azure.com") { | ||
| return Err(CompletionError::ProviderError(format!( | ||
| "Invalid Azure endpoint. Azure OpenAI Service requires a base_url ending in '.openai.azure.com'.\n\ | ||
| \n\ | ||
| Detected: {}\n\ | ||
| \n\ | ||
| If you are using Azure AI Foundry (hosting Anthropic, Llama, Mistral, etc.) or other Azure-hosted models,\n\ | ||
| use the standard OpenAI-compatible provider instead:\n\ | ||
| - Set api_type = \"openai_chat_completions\"\n\ | ||
| - Point base_url directly to your endpoint (e.g., https://<resource>.services.ai.azure.com)\n\ | ||
| - Omit the 'deployment' and 'api_version' fields\n\ | ||
| \n\ | ||
| For Azure OpenAI Service, the endpoint must follow this pattern:\n\ | ||
| https://<resource-name>.openai.azure.com", | ||
| base_url | ||
| ))); | ||
| } | ||
|
|
||
| let resource = base_url | ||
| .trim_start_matches("https://") | ||
| .trim_end_matches(".openai.azure.com"); | ||
|
|
||
| let deployment = provider_config | ||
| .deployment | ||
| .as_ref() | ||
| .ok_or_else(|| CompletionError::ProviderError( | ||
| "Azure deployment name is required. Example: 'gpt-4o', 'gpt-35-turbo', etc.\n\ | ||
| This is the deployment name you created in the Azure Portal for your OpenAI model." | ||
| .to_string() | ||
| ))?; | ||
|
|
||
| let api_version = provider_config | ||
| .api_version | ||
| .as_ref() | ||
| .ok_or_else(|| CompletionError::ProviderError( | ||
| "Azure API version is required. Example: '2024-12-01-preview'\n\ | ||
| Find available API versions in the Azure OpenAI documentation." | ||
| .to_string() | ||
| ))?; | ||
|
|
||
| let endpoint = format!( | ||
| "https://{}.openai.azure.com/openai/deployments/{}/chat/completions?api-version={}", | ||
| resource, deployment, api_version |
Contributor
There was a problem hiding this comment.
Parse the Azure origin instead of slicing it.
Line 185 assumes the value starts with https://, but Lines 166-181 only validate the suffix. A config like http://resource.openai.azure.com will pass the guard and then produce https://http://resource.openai.azure.com/... at Line 207. Please validate/extract the origin once and reuse that helper in both Azure paths so the request URL can't drift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/llm/model.rs` around lines 163 - 208, The code slices base_url (variable)
assuming a leading "https://" and later reconstructs the endpoint, which breaks
for inputs like "http://..."; update the Azure-path logic to parse and validate
the origin once (e.g., using Url::parse on provider_config.base_url), assert the
scheme is "https" and the host ends_with ".openai.azure.com", then derive a
canonical origin (scheme + "://" + host) and reuse that origin when building the
endpoint string instead of manually trimming/adding "https://"; change the
resource/deployment/api_version usage to format!("{}
/openai/deployments/{}/chat/completions?api-version={}", origin, deployment,
api_version) so both validation and endpoint construction use the same parsed
origin.
- Security: Remove api_key from ProviderConfigResponse to prevent credential leakage - Security: Preserve existing API key when updating config with empty value - Functional: Validate and route using deployment name instead of model field - Functional: Derive azureModel from azureDeployment in test and save operations - UX: Add AbortController to prevent stale state when switching providers - UX: Normalize base URLs to accept trailing slashes - Docs: Fix README example to use correct Azure config structure
Contributor
Author
|
Addressed all CodeRabbit feedback: ✅ Security Fixes:
✅ Functional Fixes:
✅ UX Improvements:
✅ Documentation:
All changes have been tested and verified. Ready for re-review. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
interface/src/routes/Settings.tsx (1)
415-418:⚠️ Potential issue | 🟡 MinorMatch the Azure hostname check to the backend rule.
The UI still uses
includes(".openai.azure.com"), so values likehttps://foo.openai.azure.com.evil.compass local validation even thoughsrc/llm/model.rsrejects them. UseendsWith()after trimming trailing/, and update the helper copy to say “end with”.🛠️ Suggested fix
const normalizedBaseUrl = azureBaseUrl.trim().replace(/\/+$/, ''); - if (!normalizedBaseUrl.includes(".openai.azure.com")) { - setTestResult({ success: false, message: "Base URL must contain '.openai.azure.com' (e.g., https://{resource-name}.openai.azure.com)" }); + if (!normalizedBaseUrl.endsWith(".openai.azure.com")) { + setTestResult({ success: false, message: "Base URL must end with '.openai.azure.com' (e.g., https://{resource-name}.openai.azure.com)" }); return false; } @@ const normalizedBaseUrl = azureBaseUrl.trim().replace(/\/+$/, ''); - if (!normalizedBaseUrl.includes(".openai.azure.com")) { - setMessage({ text: "Base URL must contain '.openai.azure.com'", type: "error" }); + if (!normalizedBaseUrl.endsWith(".openai.azure.com")) { + setMessage({ text: "Base URL must end with '.openai.azure.com'", type: "error" }); return; } @@ - Must contain '.openai.azure.com' + Must end with '.openai.azure.com'Also applies to: 465-467, 893-895
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/routes/Settings.tsx` around lines 415 - 418, The Azure hostname validation currently uses includes(".openai.azure.com") on the normalizedBaseUrl (computed from azureBaseUrl.trim().replace(/\/+$/, '')), which lets malicious hostnames pass; change the check to use normalizedBaseUrl.endsWith(".openai.azure.com") and update the user-facing message passed to setTestResult to say "Base URL must end with '.openai.azure.com' (e.g., https://{resource-name}.openai.azure.com)". Make the same edits for the other occurrences that use the same pattern in this file (the other normalizedBaseUrl checks and their setTestResult messages) so the UI validation matches src/llm/model.rs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@interface/src/routes/Settings.tsx`:
- Around line 736-771: Reset Azure form fields before hydrating from disk and
also clear them when the fetch returns without success or missing keys: in the
provider selection handler (the block that checks provider.id === "azure" and
calls api.getProviderConfig), call setAzureBaseUrl(""), setAzureApiVersion(""),
setAzureDeployment(""), and setKeyInput("") immediately before starting the
request to guarantee a clean state; then, in the .then((result) => { ... })
branch, only set each field if result.success is true and the specific property
(result.base_url, result.api_version, result.deployment) is present, otherwise
leave the cleared values, and also add an explicit else (or handle
result.success === false) to clear the fields if the API returns success: false
to avoid carrying forward stale values from previous edits.
- Around line 1011-1047: The Azure-specific footer should render whenever
editingProvider === "azure" (not only when isConfigured), with the Remove button
itself gated by isConfigured(editingProvider) and Remove's success handler
closing the dialog; change the JSX conditional to check editingProvider ===
"azure" to render the Azure footer, inside it render the Remove <Button> only if
isConfigured(editingProvider ?? "") and ensure removeMutation's onSuccess calls
handleClose (or call handleClose after mutate resolves), and keep the Save
<Button> disabled logic using the Azure fields (disable when !keyInput.trim() ||
!azureBaseUrl.trim() || !azureApiVersion.trim() || !azureDeployment.trim()) for
Azure, while for non-Azure use the existing !keyInput.trim() ||
!modelInput.trim() check; references: editingProvider, isConfigured,
removeMutation, updateMutation, handleSave, handleClose, keyInput, modelInput,
azureBaseUrl, azureApiVersion, azureDeployment.
---
Duplicate comments:
In `@interface/src/routes/Settings.tsx`:
- Around line 415-418: The Azure hostname validation currently uses
includes(".openai.azure.com") on the normalizedBaseUrl (computed from
azureBaseUrl.trim().replace(/\/+$/, '')), which lets malicious hostnames pass;
change the check to use normalizedBaseUrl.endsWith(".openai.azure.com") and
update the user-facing message passed to setTestResult to say "Base URL must end
with '.openai.azure.com' (e.g., https://{resource-name}.openai.azure.com)". Make
the same edits for the other occurrences that use the same pattern in this file
(the other normalizedBaseUrl checks and their setTestResult messages) so the UI
validation matches src/llm/model.rs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e19e2358-1b3c-4a90-bfc8-4fb4465cb292
📒 Files selected for processing (4)
README.mdinterface/src/api/client.tsinterface/src/routes/Settings.tsxsrc/api/providers.rs
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/api/providers.rs
- Construct full model string (azure/{deployment}) before validation
- model_matches_provider expects provider/model format, not just deployment name
- Fixes error: "Deployment 'gpt-5.2' does not match provider 'azure'"
Contributor
Author
|
🔧 Additional Fix Applied: Fixed a validation bug where the deployment name was being passed directly to instead of the full model string (). Before: ❌ This resolves the error: "Deployment 'gpt-5.2' does not match provider 'azure'" when saving the configuration. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/api/providers.rs (3)
838-848: Consider passing the request directly instead of reconstructing it.The
ProviderUpdateRequestis already owned, so you can passrequestdirectly toupdate_azure_providerinstead of constructing a new instance with identical fields.♻️ Simpler delegation
if normalized_provider == "azure" { - let azure_request = ProviderUpdateRequest { - provider: request.provider, - api_key: request.api_key, - model: request.model, - base_url: request.base_url, - api_version: request.api_version, - deployment: request.deployment, - }; - return update_azure_provider(state, azure_request, &normalized_model).await; + return update_azure_provider(state, request, &normalized_model).await; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/providers.rs` around lines 838 - 848, The code reconstructs an identical ProviderUpdateRequest (azure_request) then passes it to update_azure_provider; instead, forward the existing owned request directly: remove the azure_request construction and call update_azure_provider(state, request, &normalized_model). Ensure you don't accidentally borrow when update_azure_provider expects ownership; if it expects a reference, pass &request instead and drop the redundant ProviderUpdateRequest creation.
1240-1285: Consider extracting shared Azure validation logic.The validation for
base_url,api_version, anddeploymentis duplicated betweenupdate_azure_provider(lines 928-963) andtest_provider_model(lines 1241-1285). Extracting this into a helper function would reduce duplication and ensure consistent validation behavior.♻️ Example helper
struct AzureValidatedFields { base_url: String, api_version: String, deployment: String, } fn validate_azure_fields( base_url: Option<&String>, api_version: Option<&String>, deployment: Option<&String>, ) -> Result<AzureValidatedFields, String> { let base_url = base_url.ok_or("Base URL is required")?; let normalized = base_url.trim().trim_end_matches('/'); if !normalized.ends_with(".openai.azure.com") { return Err("Base URL must end with .openai.azure.com".into()); } // ... similar for api_version and deployment Ok(AzureValidatedFields { /* ... */ }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/providers.rs` around lines 1240 - 1285, Duplicate Azure validation in test_provider_model and update_azure_provider should be extracted into a single helper; add a new function validate_azure_fields (returning an AzureValidatedFields struct with normalized base_url, api_version, deployment) that takes Option<&String> for each field, performs the same trimming, regex checks (reuse the same regexes), maps regex compile failures to StatusCode::INTERNAL_SERVER_ERROR and validation failures to Err with a descriptive message (so callers can convert to BAD_REQUEST/ProviderModelTestResponse), then replace the duplicated blocks in update_azure_provider and test_provider_model with calls to validate_azure_fields and use the returned normalized fields for further logic.
1044-1058: Compute the routing model string once.The
format!("azure/{}", normalized_deployment)expression is repeated 5 times. Consider computing it once before the routing block.♻️ Reduce repetition
+ let routing_model = format!("azure/{}", normalized_deployment); + if let Some(routing_table) = defaults .get_mut("routing") .and_then(|item| item.as_table_mut()) { if routing_table.get("channel").is_none() { - routing_table["channel"] = toml_edit::value(format!("azure/{}", normalized_deployment)); + routing_table["channel"] = toml_edit::value(&routing_model); } // ... same for other fields }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/providers.rs` around lines 1044 - 1058, Compute the repeated routing model string once and reuse it: create a local variable (e.g., let routing_model = format!("azure/{}", normalized_deployment)) before the block that updates routing_table, then replace the five occurrences of format!("azure/{}", normalized_deployment) in the routing_table assignments for the "channel", "branch", "worker", "compactor", and "cortex" keys with toml_edit::value(routing_model.clone()) (or &routing_model as appropriate) so the value is constructed only once while preserving current behavior in the routing_table updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/providers.rs`:
- Around line 1243-1251: The validation in test_provider_model currently checks
base_url.trim().ends_with(".openai.azure.com") without removing trailing
slashes; update the check to first normalize base_url by trimming whitespace and
stripping trailing '/' characters (e.g., let normalized =
base_url.trim().trim_end_matches('/')) and then call
normalized.ends_with(".openai.azure.com"); use normalized for any subsequent
logic or error messages so the trailing-slash case is accepted consistently
(reference test_provider_model, ProviderModelTestResponse, and
request.provider/request.model).
- Around line 930-935: The base_url validation currently checks
base_url.ends_with(".openai.azure.com") before normalizing, so a trailing slash
causes false negatives; update the logic to first derive normalized_base_url by
trimming whitespace and any trailing '/' (e.g., with
base_url.trim().trim_end_matches('/')) and then perform the suffix check against
normalized_base_url, returning ProviderUpdateResponse on failure as before, and
ensure you use normalized_base_url (not the original base_url) when persisting
the value to TOML (the variable referenced as normalized_base_url and the
ProviderUpdateResponse in this function).
---
Nitpick comments:
In `@src/api/providers.rs`:
- Around line 838-848: The code reconstructs an identical ProviderUpdateRequest
(azure_request) then passes it to update_azure_provider; instead, forward the
existing owned request directly: remove the azure_request construction and call
update_azure_provider(state, request, &normalized_model). Ensure you don't
accidentally borrow when update_azure_provider expects ownership; if it expects
a reference, pass &request instead and drop the redundant ProviderUpdateRequest
creation.
- Around line 1240-1285: Duplicate Azure validation in test_provider_model and
update_azure_provider should be extracted into a single helper; add a new
function validate_azure_fields (returning an AzureValidatedFields struct with
normalized base_url, api_version, deployment) that takes Option<&String> for
each field, performs the same trimming, regex checks (reuse the same regexes),
maps regex compile failures to StatusCode::INTERNAL_SERVER_ERROR and validation
failures to Err with a descriptive message (so callers can convert to
BAD_REQUEST/ProviderModelTestResponse), then replace the duplicated blocks in
update_azure_provider and test_provider_model with calls to
validate_azure_fields and use the returned normalized fields for further logic.
- Around line 1044-1058: Compute the repeated routing model string once and
reuse it: create a local variable (e.g., let routing_model = format!("azure/{}",
normalized_deployment)) before the block that updates routing_table, then
replace the five occurrences of format!("azure/{}", normalized_deployment) in
the routing_table assignments for the "channel", "branch", "worker",
"compactor", and "cortex" keys with toml_edit::value(routing_model.clone()) (or
&routing_model as appropriate) so the value is constructed only once while
preserving current behavior in the routing_table updates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6a3a6378-55e9-4f13-b22f-995ef3903366
📒 Files selected for processing (1)
src/api/providers.rs
- Remove key validation from early return in handleTestModel and handleSave - Add Azure-specific validation that checks for empty API key only when provider is Azure - Show clear error message: "API key is required for Azure" when key is empty - Allow users to enter a key and then test/save, improving UX while maintaining security
Contributor
Author
|
🎨 UX Improvement: Fixed an issue where the Test Model and Save buttons were disabled when the API key field was blank. Before:
After:
This improves the user experience while maintaining security (keys are still not auto-populated). |
- When testing Azure model with empty API key, read stored key from config.toml - Allows users to test existing configuration without re-entering the key - Maintains security: key is never returned to client, only used for testing - For non-Azure providers, still require explicit API key in request - Clear error message when no key is available: "API key is required but not provided"
- Remove frontend validation blocking test when API key is blank - Backend will use stored key if incoming key is empty - Allows testing existing Azure configuration without re-entering credentials
- Remove frontend validation blocking save when API key is blank - Remove !keyInput.trim() from Save button disabled condition - Backend update_azure_provider already handles empty keys by using stored key - Consistent with handleTestModel fix for complete Azure UX improvement
Contributor
Author
|
- Add missing keyInput validation for Azure in handleTestModel/handleSave - Change base URL validation from .includes() to .endsWith() to match backend - Reset Azure form fields before hydrating from config to prevent stale state - Show Azure footer for all Azure edits, not just configured providers - Gate Remove button on isConfigured, close dialog on successful remove - Add keyInput to Save button disabled check - Normalize trailing slash before base_url suffix check in providers.rs - Return proper JSON error instead of raw StatusCode when API key missing - Validate HTTPS scheme in model.rs Azure endpoint construction Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…mat) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
jamiepine
approved these changes
Apr 2, 2026
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
2 participants
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.
Summary
Changes
Rust Backend
update_azure_provider()handler with comprehensive validationtest_provider_model()to handle Azure provider typeupdate_provider()to route Azure requests correctlyFrontend
Documentation
Technical Details
azure/<deployment-name>(e.g.,azure/gpt-4o,azure/gpt-5.2).openai.azure.comYYYY-MM-DDorYYYY-MM-DD-previewFiles Changed
src/api/providers.rs- Azure handlers and validation logicsrc/api/server.rs- Route registrationsrc/config/types.rs- Azure config types and schemassrc/config/providers.rs- Azure provider defaults and initializationsrc/llm/model.rs- Azure model initializationinterface/src/routes/Settings.tsx- Azure UI componentsinterface/src/api/client.ts- Azure API callsdocs/content/docs/(configuration)/config.mdx- Azure configuration docsdocs/content/docs/(core)/routing.mdx- Azure routing examplesREADME.md- Azure configuration exampleTest Plan
cargo test --libjust preflightjust gate-prVerification Commands
cargo test azure just preflight just gate-prBreaking Changes
None
Notes
Azure OpenAI provider follows the same pattern as other custom providers but includes Azure-specific validation and configuration requirements. The implementation is fully backward compatible with existing providers.