feat(action): validate Azure setup instead of reporting a missing key - #70
feat(action): validate Azure setup instead of reporting a missing key#70theoephraim wants to merge 1 commit into
Conversation
Azure models reach validateAgentApiKey as raw models.dev specifiers (azure/<model-id>), so they never hit the Bedrock/Vertex routing branches and fall through to the generic "no API key found" error. That copy is wrong for both Azure failure modes: a missing AZURE_RESOURCE_NAME, and a deployment whose name doesn't match the model id — the latter tells users to add a key they already added. Add validateAzureSetup alongside the Bedrock and Vertex validators. It names whichever env var is absent, and when both are set it explains that the provider addresses a deployment and uses the model id as the deployment name, so the deployment must be renamed to match. Covers both models.dev Azure providers (azure, azure-cognitive-services), which differ only in env-var prefix. No-ops for every other provider, so the generic error is unchanged elsewhere.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 21fa4cf. Configure here.
| missing, | ||
| envVars, | ||
| }) | ||
| ); |
There was a problem hiding this comment.
Azure errors skip PR comment rendering
Medium Severity
validateAzureSetup throws messages that lack both the missing-key marker and a pass-through marker like MODEL_ACCESS_MARKER, so renderRunError treats them as generic failures. The PR comment collapses to a one-line logs link, and the job summary frames the setup guidance as an unexpected error. Before this change, Azure failures went through buildMissingApiKeyError and were mirrored onto the PR comment.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 21fa4cf. Configure here.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — a dedicated Azure setup validator that replaces the generic "missing key" error when an azure/* model is unauthorized, so the two real Azure failure modes (a missing env var vs. a deployment-name mismatch) each get accurate copy.
- Add
validateAzureSetup+buildAzureSetupError— sited beside the Bedrock/Vertex validators; throws for Azure providers and returns (no-op) for everything else, leaving the generic path untouched. AZURE_PROVIDERSmap — covers both models.dev Azure providers (azure,azure-cognitive-services), which differ only in env-var prefix.- Wire into the opencode branch — runs after the
authorized.has(model)check invalidateAgentApiKey, deriving the provider from the model prefix. - Six tests in
utils/apiKeys.test.tscovering authorized success, each missing var, the deployment-mismatch copy, the cognitive-services prefix, and that non-Azure providers stay on the generic error.
I verified the surrounding integration beyond the diff: the provider/deployment split is safe because the earlier !params.model.includes("/") guard (apiKeys.ts:236) guarantees a / before this branch runs, and the new error deliberately omits the MISSING_KEY_MARKER, so isApiKeyAuthError correctly leaves it alone rather than re-routing it through the "rotate your key" copy. Test isolation also holds — the /_API_KEY$/ strip pattern already clears AZURE_API_KEY between cases. One awareness note below; nothing blocking.
ℹ️ Detailed Azure guidance lands in the Actions job summary, not the PR comment
Because the Azure setup error carries no MISSING_KEY_MARKER, it flows through the generic-failure branch in renderRunError — the full deployment-mismatch guidance renders in the GitHub Actions job summary, while the PR progress comment collapses to a one-line **Run failed.** [View the logs →]. This is identical to how the existing Bedrock/Vertex setup errors behave, so it's consistent, not a regression. Worth confirming it matches intent, since the generic missing-key path (which does carry the marker) surfaces its rich copy directly in the PR comment.
Technical details
# Azure setup error surfaces in job summary, not PR comment
## Affected sites
- `utils/runErrorRenderer.ts:227-266` — `isApiKeyAuthError(apiKeySource)` is false for the Azure error (no `MISSING_KEY_MARKER`), so it falls through to `formatGenericFailure` → `summary` = full message, `comment` = `formatMinimalFailureComment` (one-liner).
- `utils/apiKeys.ts:118-124` — deployment-mismatch copy that only reaches the `summary` surface.
## Required outcome
- No change required if parity with Bedrock/Vertex is the intended behavior.
## Open questions for the human
- Should the multi-var setup errors (Azure/Bedrock/Vertex) surface their detailed copy in the PR comment too, or is the job-summary-only placement acceptable? This is a shared design decision beyond this PR's scope.Claude Opus (free via Pullfrog for OSS) ｜ 𝕏
|
Closing: superseded by the first-class Azure support that landed in 5f2ce64 (#1226). Its validateAzureSetup covers what this PR did and goes further, also requiring the deployment name and token limits. Great to see it shipped! |
1 participant
feel free to toss or reimplement. I got azure wired up successfully, but this is what came out of a review about improving pullfrog after doing so. I will note that azure foundry is pretty awkward, so it's easy to get this wrong... So better error messaging may help someone.
Context
Azure-hosted OpenAI models already work today with no changes to Pullfrog:
resolveModelpasses a slashed non-curated value through as a raw models.dev specifier, the BYOK gate isopencode modelsoutput rather than a fixed allowlist, and models.dev ships anazureprovider (AZURE_RESOURCE_NAME+AZURE_API_KEY) carrying the GPT models. We're running our PR reviews onazure/gpt-5.6-solthis way.This PR doesn't add Azure support. It makes the failure mode legible when the setup is wrong.
Problem
Because Azure has no curated alias, it carries no
routingdiscriminant, so it skips the Bedrock and Vertex branches invalidateAgentApiKeyand lands on the genericbuildMissingApiKeyError. That message is wrong for both ways Azure fails:AZURE_API_KEYbut notAZURE_RESOURCE_NAMEand you're told no API key was found, pointing at the key you just added.@ai-sdk/azureaddresses a deployment, not a model, and uses the model id as the deployment name. Name your deploymentsol-reviewinstead ofgpt-5.6-soland it never enters the authorized set, so credentials that are completely fine report as a missing key.The second one cost us the most time, and nothing in the output points at it.
Change
validateAzureSetup, sited beside the two existing validators and following their shape. It names whichever var is absent; when both are present it explains the deployment-name coupling and says exactly what the deployment must be called.Rendered, for
azure/gpt-5.6-solwith both vars set:Both models.dev Azure providers are covered (
azure,azure-cognitive-services), since they differ only in env-var prefix. The validator returns without throwing for any non-Azure provider, so the generic error path is untouched elsewhere — there's a test pinning that.Deliberately no
docs.pullfrog.com/azurelink, since that page doesn't exist yet. Happy to write one as a follow-up if you'd take it.Tests
Six cases in
utils/apiKeys.test.ts. Verified the four behavioral ones fail when the hook is disabled.pnpm typecheckclean,utils/282/282. Didn't run the full suite (no GitHub App credentials locally).Branched off
main, independent of #69.Note
Low Risk
Changes are limited to pre-run validation error messaging in
utils/apiKeys.tswith no auth or runtime behavior changes; non-Azure paths are explicitly no-ops.Overview
When an OpenCode run uses an unauthorized
azure/...orazure-cognitive-services/...model,validateAgentApiKeyno longer falls through to the generic “no API key found” message. It now runsvalidateAzureSetup, aligned with the existing Bedrock/Vertex setup validators.Missing
AZURE_RESOURCE_NAME/AZURE_API_KEY(or the cognitive-services equivalents) produce errors that name the specific env vars and how to set them in GitHub Actions. If both are set but the model still isn’t authorized, the error calls out a likely deployment name mismatch (Azure uses the model id as the deployment name) instead of blaming a missing key. Non-Azure providers still get the generic missing-key path.Tests cover authorized success, missing vars, deployment mismatch copy, both Azure provider prefixes, and that other providers are unchanged.
Reviewed by Cursor Bugbot for commit 21fa4cf. Bugbot is set up for automated code reviews on this repo. Configure here.