Add Azure OpenAI provider configuration#558
Open
Random-Word wants to merge 5 commits into
Open
Conversation
|
Someone is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
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:
📝 WalkthroughWalkthroughAdds Azure OpenAI support across docs, CLI, config detection (including legacy base-URL parsing), provider wiring (Azure auth/version and GPT‑5 payload key), and tests verifying detection and request/header behavior. ChangesAzure OpenAI Provider Integration
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Config
participant Provider
participant AzureAPI
CLI->>Config: loadConfig()/runOnboarding (env)
Config->>Provider: detectProviderForEnv -> OpenAI config (azure baseURL/deployment)
Provider->>AzureAPI: HTTP request (api-key header or Authorization, api-version, payload)
AzureAPI-->>Provider: response
Provider-->>CLI: summarize result / error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/cli/doctor-diagnostics.ts (1)
89-97:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRequire the full Azure config before this diagnostic passes.
Adding
AZURE_OPENAI_API_KEYto the generic key list makesrealProviderKeys()treat the Azure key alone as sufficient, sono-llm-provider-keycan pass even when runtime still falls back to noop becauseAZURE_OPENAI_ENDPOINT+AZURE_OPENAI_DEPLOYMENT(orAZURE_OPENAI_BASE_URL) are missing. The help text should point at that full Azure tuple too.🔧 Suggested fix
export function realProviderKeys(env: Record<string, string>): string[] { return PROVIDER_KEY_NAMES.filter((k) => { const v = (env[k] ?? "").trim(); if (!v) return false; if (PLACEHOLDER_VALUES.has(v.toLowerCase())) return false; // Reject values that are just dots/placeholders like "xxxx-xxxx". if (/^x+$/i.test(v.replace(/[-_]/g, ""))) return false; + if (k === "AZURE_OPENAI_API_KEY") { + const hasBaseUrl = !!(env["AZURE_OPENAI_BASE_URL"] ?? "").trim(); + const hasEndpoint = !!(env["AZURE_OPENAI_ENDPOINT"] ?? "").trim(); + const hasDeployment = !!(env["AZURE_OPENAI_DEPLOYMENT"] ?? "").trim(); + if (!hasBaseUrl && !(hasEndpoint && hasDeployment)) return false; + } return true; }); }- "Set at least one of: ANTHROPIC_API_KEY, AZURE_OPENAI_API_KEY, OPENAI_API_KEY, GEMINI_API_KEY, " + - "OPENROUTER_API_KEY, MINIMAX_API_KEY. The daemon picks the first that resolves " + + "Set at least one of: ANTHROPIC_API_KEY, OPENAI_API_KEY, GEMINI_API_KEY, GOOGLE_API_KEY, " + + "OPENROUTER_API_KEY, MINIMAX_API_KEY, or Azure OpenAI (`AZURE_OPENAI_API_KEY` plus " + + "`AZURE_OPENAI_ENDPOINT` + `AZURE_OPENAI_DEPLOYMENT`, or `AZURE_OPENAI_BASE_URL`). The daemon picks the first that resolves " + "to a real (non-placeholder) value at startup.",Also applies to: 201-203
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/doctor-diagnostics.ts` around lines 89 - 97, The diagnostic currently treats AZURE_OPENAI_API_KEY as a standalone valid provider key; update the logic so Azure requires the full config tuple instead: remove AZURE_OPENAI_API_KEY from the generic PROVIDER_KEY_NAMES list and add explicit Azure validation in realProviderKeys() (or the diagnostic function) that only marks Azure as available when AZURE_OPENAI_API_KEY is present AND (AZURE_OPENAI_ENDPOINT and AZURE_OPENAI_DEPLOYMENT) or AZURE_OPENAI_BASE_URL are present; also update the user-facing help text emitted by the no-llm-provider-key diagnostic to mention the required Azure tuple (API key + endpoint + deployment/base URL) and mirror this change in the other diagnostic usages of the provider list (the other occurrences of PROVIDER_KEY_NAMES / related checks).src/functions/summarize.ts (1)
247-252:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReturn the exact supported config here instead of provider shorthands.
This reason string suggests
AZURE_OPENAI, which is not an env var, and it still omitsOPENAI_API_KEY. Because users see this on the failure path, it should reference the exact accepted settings, including Azure's endpoint/deployment requirement.✏️ Suggested wording
- reason: - "No LLM provider key set; Summarize is a no-op. Set ANTHROPIC_API_KEY (or AZURE_OPENAI/GEMINI/OPENROUTER/MINIMAX) in ~/.agentmemory/.env to enable.", + reason: + "No LLM provider configured; Summarize is a no-op. Set ANTHROPIC_API_KEY, OPENAI_API_KEY, GEMINI_API_KEY/GOOGLE_API_KEY, OPENROUTER_API_KEY, MINIMAX_API_KEY, or AZURE_OPENAI_API_KEY + AZURE_OPENAI_ENDPOINT + AZURE_OPENAI_DEPLOYMENT in ~/.agentmemory/.env to enable.",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/functions/summarize.ts` around lines 247 - 252, The failure message in the return object inside summarize (src/functions/summarize.ts) uses shorthand provider names and omits/incorrectly names env vars; update the reason string to list the exact accepted environment variables and Azure requirements (for example: ANTHROPIC_API_KEY, OPENAI_API_KEY, AZURE_OPENAI_ENDPOINT and AZURE_OPENAI_API_KEY (plus any AZURE_OPENAI_DEPLOYMENT if your code expects a deployment name), OPENROUTER_API_KEY, GEMINI_API_KEY, MINIMAX_API_KEY) and mention that Azure requires both endpoint and key (or deployment where applicable) so users can set the precise config instead of provider shorthands.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cli.ts`:
- Around line 1462-1465: Update the hint text for the "LLM provider" doctor
entry (the object with name "LLM provider") and the other similar entry around
lines 1898-1900 to list the actual environment variables the CLI reads: include
ANTHROPIC_API_KEY, OPENAI_API_KEY (public OpenAI), AZURE_OPENAI_KEY plus
AZURE_OPENAI_ENDPOINT and AZURE_OPENAI_DEPLOYMENT (for Azure), and the
provider-specific keys such as GEMINI_API_KEY, OPENROUTER_API_KEY, and
MINIMAX_API_KEY; ensure the hint explicitly mentions the required Azure endpoint
and deployment variables rather than the generic AZURE_OPENAI name.
In `@src/cli/onboarding.ts`:
- Around line 63-64: The fallback .env skeleton only seeds AZURE_OPENAI_API_KEY
for the "azure-openai" provider (the array entry with value "azure-openai" /
envKey "AZURE_OPENAI_API_KEY"), so update the code that builds the fallback env
entries to also include the Azure endpoint/base and deployment variables (e.g.,
AZURE_OPENAI_API_BASE or AZURE_OPENAI_ENDPOINT and AZURE_OPENAI_DEPLOYMENT or
AZURE_OPENAI_DEPLOYMENT_NAME) wherever the provider list/skeleton is defined
(the "azure-openai" entry and the corresponding fallback-generation logic
referenced around the other occurrence at lines ~234-238) so users get the full
set of required env vars when selecting Azure OpenAI.
---
Outside diff comments:
In `@src/cli/doctor-diagnostics.ts`:
- Around line 89-97: The diagnostic currently treats AZURE_OPENAI_API_KEY as a
standalone valid provider key; update the logic so Azure requires the full
config tuple instead: remove AZURE_OPENAI_API_KEY from the generic
PROVIDER_KEY_NAMES list and add explicit Azure validation in realProviderKeys()
(or the diagnostic function) that only marks Azure as available when
AZURE_OPENAI_API_KEY is present AND (AZURE_OPENAI_ENDPOINT and
AZURE_OPENAI_DEPLOYMENT) or AZURE_OPENAI_BASE_URL are present; also update the
user-facing help text emitted by the no-llm-provider-key diagnostic to mention
the required Azure tuple (API key + endpoint + deployment/base URL) and mirror
this change in the other diagnostic usages of the provider list (the other
occurrences of PROVIDER_KEY_NAMES / related checks).
In `@src/functions/summarize.ts`:
- Around line 247-252: The failure message in the return object inside summarize
(src/functions/summarize.ts) uses shorthand provider names and omits/incorrectly
names env vars; update the reason string to list the exact accepted environment
variables and Azure requirements (for example: ANTHROPIC_API_KEY,
OPENAI_API_KEY, AZURE_OPENAI_ENDPOINT and AZURE_OPENAI_API_KEY (plus any
AZURE_OPENAI_DEPLOYMENT if your code expects a deployment name),
OPENROUTER_API_KEY, GEMINI_API_KEY, MINIMAX_API_KEY) and mention that Azure
requires both endpoint and key (or deployment where applicable) so users can set
the precise config instead of provider shorthands.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d14a2cea-2270-4107-a783-33d38438826c
📒 Files selected for processing (13)
.env.exampleREADME.mdsrc/cli.tssrc/cli/doctor-diagnostics.tssrc/cli/iii-console-install.tssrc/cli/onboarding.tssrc/config.tssrc/functions/summarize.tssrc/providers/index.tssrc/providers/openai.tstest/config-azure-openai.test.tstest/iii-console-install.test.tstest/openai-shared.test.ts
Adds Azure OpenAI API-key configuration for LLM compression and summarization, including endpoint/deployment detection, OpenAI provider key precedence, docs, onboarding, doctor hints, and focused tests. Documents that Microsoft Entra ID / DefaultAzureCredential auth is not implemented yet; this PR supports Azure OpenAI API-key auth only. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Random-Word
force-pushed
the
feature/azure-openai-support
branch
from
04f4509 to
fdd8cb8
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli/doctor-diagnostics.ts (1)
89-97:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t treat
AZURE_OPENAI_API_KEYalone as a valid LLM configuration.Doctor now reports success when only
AZURE_OPENAI_API_KEYis set, but runtime Azure detection also requires endpoint/base URL and deployment/model. This can incorrectly pass diagnostics while provider still resolves to noop.Suggested fix direction
check: async () => { if (!effects.envFileExists()) { return { ok: false, detail: "env file missing (run env-missing fix first)" }; } const env = effects.readEnvFile(); const real = realProviderKeys(env); + const hasAzureKey = real.includes("AZURE_OPENAI_API_KEY"); + const hasAzureEndpoint = + Boolean((env["AZURE_OPENAI_ENDPOINT"] ?? "").trim()) || + Boolean((env["AZURE_OPENAI_BASE_URL"] ?? "").trim()); + const hasAzureDeployment = + Boolean((env["AZURE_OPENAI_DEPLOYMENT"] ?? "").trim()) || + Boolean((env["AZURE_OPENAI_MODEL"] ?? "").trim()); + const hasUsableAzure = hasAzureKey && hasAzureEndpoint && hasAzureDeployment; + const hasOtherProvider = real.some((k) => k !== "AZURE_OPENAI_API_KEY"); + const ok = hasOtherProvider || hasUsableAzure; return { - ok: real.length > 0, - detail: real.length > 0 ? `found: ${real.join(", ")}` : "no provider key set", + ok, + detail: ok ? `found: ${real.join(", ")}` : "no provider key set", }; },Also applies to: 201-203
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/doctor-diagnostics.ts` around lines 89 - 97, The diagnostics currently treat AZURE_OPENAI_API_KEY in PROVIDER_KEY_NAMES as a standalone valid LLM credential; update the doctor logic so that Azure is only considered configured when the API key plus the required Azure-specific settings are present (e.g., AZURE_OPENAI_ENDPOINT or AZURE_OPENAI_BASE_URL and an AZURE_OPENAI_DEPLOYMENT or model identifier). Modify the check that uses PROVIDER_KEY_NAMES and any related validation code (referencing PROVIDER_KEY_NAMES and the Azure-specific keys) to require the composite Azure configuration rather than only AZURE_OPENAI_API_KEY, and mirror this change in the similar validation handled around the other Azure-related checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/providers/index.ts`:
- Around line 100-105: The selection logic for openaiKey can miss an Azure base
supplied via environment; update the useAzureKey computation to also check
process.env.OPENAI_BASE_URL when config.baseURL is falsy (e.g., set useAzureKey
= config.baseURL ? detectAzure(config.baseURL) :
detectAzure(process.env.OPENAI_BASE_URL || "")); then keep the openaiKey
selection logic (azureKey, standardKey) so that when detectAzure reports Azure
you prefer azureKey over standardKey; reference the existing symbols azureKey,
standardKey, useAzureKey, openaiKey, detectAzure and getEnvVar to locate and
change the code.
---
Outside diff comments:
In `@src/cli/doctor-diagnostics.ts`:
- Around line 89-97: The diagnostics currently treat AZURE_OPENAI_API_KEY in
PROVIDER_KEY_NAMES as a standalone valid LLM credential; update the doctor logic
so that Azure is only considered configured when the API key plus the required
Azure-specific settings are present (e.g., AZURE_OPENAI_ENDPOINT or
AZURE_OPENAI_BASE_URL and an AZURE_OPENAI_DEPLOYMENT or model identifier).
Modify the check that uses PROVIDER_KEY_NAMES and any related validation code
(referencing PROVIDER_KEY_NAMES and the Azure-specific keys) to require the
composite Azure configuration rather than only AZURE_OPENAI_API_KEY, and mirror
this change in the similar validation handled around the other Azure-related
checks.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c3ab4006-9620-4545-9107-f667a6e35267
📒 Files selected for processing (11)
.env.exampleREADME.mdsrc/cli.tssrc/cli/doctor-diagnostics.tssrc/cli/onboarding.tssrc/config.tssrc/functions/summarize.tssrc/providers/index.tssrc/providers/openai.tstest/config-azure-openai.test.tstest/openai-shared.test.ts
✅ Files skipped from review due to trivial changes (3)
- .env.example
- README.md
- src/functions/summarize.ts
Uses max_completion_tokens for GPT-5 deployments while preserving max_tokens for existing models. This is required for Azure OpenAI gpt-5.4-mini deployments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/providers/openai.ts`:
- Around line 91-92: The code picks the token-limit field via
usesMaxCompletionTokens(this.model) which fails for Azure deployments with
arbitrary names; add support for an environment override by reading
process.env.OPENAI_TOKEN_LIMIT_FIELD and, if present and equal to
"max_completion_tokens" or "max_tokens", use that value to choose the body key;
otherwise fall back to the existing usesMaxCompletionTokens(this.model) check.
Update the assignment that currently sets
body[usesMaxCompletionTokens(this.model) ? "max_completion_tokens" :
"max_tokens"] = this.maxTokens to first derive fieldName from
OPENAI_TOKEN_LIMIT_FIELD (validated) or from
usesMaxCompletionTokens(this.model), then set body[fieldName] = this.maxTokens,
and ensure invalid env values are ignored or logged and do not break the
fallback.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8e718de1-4e82-4f95-a23b-a5babe09181c
📒 Files selected for processing (2)
src/providers/openai.tstest/openai-shared.test.ts
Spells out concrete provider env vars in runtime hints and selects AZURE_OPENAI_API_KEY when OPENAI_BASE_URL is Azure-shaped but provided through the environment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolves the OpenAI provider conflict while keeping Azure OpenAI GPT-5 token handling and upstream non-streaming requests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/providers/openai.ts (1)
98-99:⚠️ Potential issue | 🟠 Major | ⚡ Quick winArbitrary Azure deployment names will cause incorrect token field selection.
This issue was previously flagged: Azure deployments can use arbitrary names (e.g.,
prod-chat,my-gpt5-model) that don't match the^gpt-5...pattern inusesMaxCompletionTokens(). When a GPT-5 deployment has a non-matching name, the code sendsmax_tokensinstead ofmax_completion_tokens, causing Azure to reject the request.The past review comment (lines 98-99) provides a detailed solution using an
OPENAI_TOKEN_LIMIT_FIELDenvironment variable override with validation and fallback to pattern detection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/providers/openai.ts` around lines 98 - 99, The token-field selection code that assigns body[usesMaxCompletionTokens(this.model) ? "max_completion_tokens" : "max_tokens"] = this.maxTokens will mis-detect Azure GPT-5 deployments with arbitrary names; change the logic to first check an environment override (OPENAI_TOKEN_LIMIT_FIELD) with validation (only allow "max_tokens" or "max_completion_tokens"), use that value if valid, otherwise fall back to the existing usesMaxCompletionTokens(this.model) pattern detection, and ensure the chosen field is used when setting body[...] so Azure receives the correct token field; update any related unit tests for setTokenField behavior and document the env var.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/providers/openai.ts`:
- Around line 98-99: The token-field selection code that assigns
body[usesMaxCompletionTokens(this.model) ? "max_completion_tokens" :
"max_tokens"] = this.maxTokens will mis-detect Azure GPT-5 deployments with
arbitrary names; change the logic to first check an environment override
(OPENAI_TOKEN_LIMIT_FIELD) with validation (only allow "max_tokens" or
"max_completion_tokens"), use that value if valid, otherwise fall back to the
existing usesMaxCompletionTokens(this.model) pattern detection, and ensure the
chosen field is used when setting body[...] so Azure receives the correct token
field; update any related unit tests for setTokenField behavior and document the
env var.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f9ee3f5b-a62b-4030-aef1-51934f4e3a62
📒 Files selected for processing (3)
README.mdsrc/cli.tssrc/providers/openai.ts
✅ Files skipped from review due to trivial changes (2)
- src/cli.ts
- README.md
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
1 participant
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
AZURE_OPENAI_API_KEY,AZURE_OPENAI_ENDPOINT, andAZURE_OPENAI_DEPLOYMENT.AZURE_OPENAI_BASE_URLandAZURE_OPENAI_API_VERSION.max_completion_tokensfor GPT-5 deployments such asgpt-5.4-mini, while preservingmax_tokensfor older models..env.example, and README docs.Auth scope
This PR supports Azure OpenAI API-key auth only. Microsoft Entra ID /
DefaultAzureCredentialauth is being developed separately onfeature/azure-openai-adc.Validation
npm test -- test/config-azure-openai.test.ts test/openai-shared.test.ts test/cli-doctor-fixes.test.ts test/cli-onboarding.test.tsnpx tsdowngit diff --checkI did not run a real Azure OpenAI API-key smoke test because no
AZURE_OPENAI_API_KEYcredentials are present in the process environment or~/.agentmemory/.envon this machine. The related ADC branch successfully reached the NL2SQLgpt-5.4-minideployment and exposed the GPT-5 token parameter requirement that is now backported here.Summary by CodeRabbit
New Features
Documentation
Compatibility
Tests