Skip to content

fix: Azure OpenAI memory indexing fails for custom providers#111813

Open
vyncint wants to merge 5 commits into
openclaw:mainfrom
vyncint:fix/azure-embedding-api-version
Open

fix: Azure OpenAI memory indexing fails for custom providers#111813
vyncint wants to merge 5 commits into
openclaw:mainfrom
vyncint:fix/azure-embedding-api-version

Conversation

@vyncint

@vyncint vyncint commented Jul 20, 2026 •
edited
Loading

Copy link
Copy Markdown

Closes #111386

What Problem This Solves

Fixes an issue where users indexing memory through a custom Azure OpenAI embedding provider would receive HTTP 404 when the provider supplied api-version as a header. Azure OpenAI requires that value in the request URL query.

Why This Change Was Made

For Azure OpenAI-compatible endpoints, the embedding transport now follows the same request contract as the existing chat transport: it moves a case-insensitive api-version header into the query string at request time, preserves other configured query values, and lets an existing URL value take precedence. Non-Azure OpenAI-compatible endpoints keep their existing header behavior.

This matches the Azure OpenAI REST API contract, which defines api-version as a required query parameter. Reported by @OliPlus in #111386.

User Impact

Users can index and search memory with custom Azure OpenAI embedding deployments without switching to another embedding provider as a workaround.

Evidence

  • node scripts/run-vitest.mjs src/plugins/openai-compatible-embedding-provider.azure.test.ts src/plugins/openai-compatible-embedding-provider.test.ts -- --reporter=verbose: 2 files and 31 tests passed.
  • The regression tests cover all three Azure-compatible hostname suffixes used by the chat transport, case-insensitive header removal, preservation of existing query parameters, URL precedence, and unchanged non-Azure proxy behavior.
  • node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.core.json src/plugins/openai-compatible-embedding-provider.azure.test.ts src/plugins/openai-compatible-embedding-provider.ts: passed.
  • Targeted oxfmt --check and git diff --check: passed.
  • Repository autoreview --mode uncommitted: clean, with no accepted/actionable findings; reviewer confidence 0.94.
  • Upstream CI passed every patch-relevant job. The only source failure was an unrelated PR-wrapper test in checks-node-compact-small-1: 1 failed, 1,291 passed, and 4 skipped. It reproduced across two PR-context runs while the same shard passed on the exact base commit; the second failed status is the aggregate ci-gate reflecting that shard.

AI-assisted: Codex helped research the issue, implement the fix, add tests, and prepare this PR. I reviewed the final diff and understand the behavior: request headers are cloned and normalized only for recognized Azure hosts, an existing URL api-version wins, and non-Azure endpoints are unchanged. A redacted session transcript is included below.

Agent Transcript

Redacted Codex session transcript 
source: [LOCAL_SESSION]
redaction: local paths, email, auth details, and raw tool output removed
omitted: unrelated repository-candidate exploration and PR-account setup

[user]
Please help me find the open source repo which accept AI coding contribute then create one contribute

[assistant]
OpenClaw was selected because its contributor guide explicitly welcomes AI-assisted PRs with disclosure. Issue #111386 was open, unassigned, and had no competing PR; current source inspection confirmed the raw embedding request still sent Azure's api-version only as a header.

The implementation moves api-version only for the same three recognized Azure OpenAI-compatible host suffixes used by the chat transport. It preserves other query parameters, lets an existing URL api-version win, removes the header case-insensitively, and leaves non-Azure endpoints unchanged.

A mocked regression suite covers all three Azure host suffixes, URL precedence, header removal, and non-Azure behavior without requiring Azure credentials. The combined focused suite passed 31 tests across 2 files; targeted type-aware lint, formatting, and diff checks also passed.

OpenClaw's repository-local autoreview ran on the uncommitted patch. After replacing a credential-shaped test fixture with an explicitly synthetic value, the isolated reviewer returned no accepted or actionable findings and rated the patch correct with 0.94 confidence.

Before publishing, the branch was rebased onto the latest origin/main. The two upstream commits were unrelated to the touched provider surface, and the rebased file contents matched the reviewed commit byte-for-byte.

Copilot AI review requested due to automatic review settings July 20, 2026 11:46
Copilot AI reviewed Jul 20, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes Azure OpenAI–compatible embedding requests in the OpenAI-compatible embedding provider by moving a configured api-version header into the request URL query string for recognized Azure hostnames, matching Azure’s required contract and preventing 404s during memory indexing/search.

Changes:

  • Add Azure hostname detection + request-target normalization that (a) preserves existing query params, (b) prefers an existing URL api-version, and (c) removes the api-version header when moved.
  • Route embedding POSTs through the normalized { url, headers } target instead of directly using ${baseUrl}/embeddings.
  • Add a dedicated Azure regression test suite covering the three Azure-compatible hostname suffixes, URL precedence, query preservation, and non-Azure behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/plugins/openai-compatible-embedding-provider.ts Normalizes Azure-compatible embedding request URL/headers so api-version is sent as a query param (while preserving existing query values and keeping non-Azure behavior unchanged).
src/plugins/openai-compatible-embedding-provider.azure.test.ts Adds focused regression tests for Azure host suffix handling, api-version query precedence, header removal, and non-Azure passthrough behavior.

@vyncint
vyncint force-pushed the fix/azure-embedding-api-version branch from e6a7472 to 7d8b82b Compare July 20, 2026 11:55
@BradGroux BradGroux mentioned this pull request Jul 20, 2026
Draft
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jul 20, 2026
@clawsweeper

clawsweeper Bot commented Jul 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 21, 2026, 4:55 AM ET / 08:55 UTC.

Summary
The PR converts an Azure-compatible embedding provider's configured api-version header into a URL query parameter for recognized Azure host suffixes and adds focused request-normalization tests.

PR surface: Source +42, Tests +122. Total +164 across 2 files.

Reviewed head: c3f5e3b49c854d208501ed15e4bf6cf377dd5a86

Reproducibility: no. live reproduction was run in this read-only review, but the current-main request construction and the reported Azure configuration make the failure path source-reproducible with high confidence.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: src/plugins/openai-compatible-embedding-provider.azure.test.ts, unknown-data-model-change: src/plugins/openai-compatible-embedding-provider.azure.test.ts, unknown-data-model-change: src/plugins/openai-compatible-embedding-provider.ts, vector/embedding metadata: src/plugins/openai-compatible-embedding-provider.azure.test.ts, vector/embedding metadata: src/plugins/openai-compatible-embedding-provider.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add a redacted successful openclaw memory index run against the Azure custom provider, ideally with a request URL or runtime trace showing the query parameter.
  • [P1] Update the PR body with that evidence; ClawSweeper should re-review automatically, or a maintainer can request @clawsweeper re-review if needed.

Proof guidance:

  • [P1] Needs real behavior proof before merge: Focused mocked tests and CI claims support request construction, but the PR still lacks redacted after-fix evidence from a real Azure memory-indexing setup; add a live run, runtime log, or request trace with secrets and private endpoint details removed. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] A mocked URL assertion cannot confirm that a real Azure deployment accepts the final request and that memory indexing completes with the contributor's configured custom provider.

Maintainer options:

  1. Decide the mitigation before merge
    Keep the Azure-only normalization and focused tests, then add a redacted real Azure openclaw memory index and search/index result or an equivalent runtime request trace showing api-version in the URL and successful embeddings.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] The remaining merge blocker is contributor-supplied real behavior proof, which an automated repair lane cannot safely create or validate on the contributor's Azure setup.

Security
Cleared: The patch changes only request URL/header normalization for recognized Azure hosts, preserves the existing SSRF-guarded fetch path, and introduces no dependency, secret, workflow, or permission change.

Review details

Best possible solution:

Keep the Azure-only normalization and focused tests, then add a redacted real Azure openclaw memory index and search/index result or an equivalent runtime request trace showing api-version in the URL and successful embeddings.

Do we have a high-confidence way to reproduce the issue?

No live reproduction was run in this read-only review, but the current-main request construction and the reported Azure configuration make the failure path source-reproducible with high confidence.

Is this the best way to solve the issue?

Yes, conditionally: Azure-host-scoped URL normalization matches the documented API contract and preserves non-Azure behavior, but a real after-fix Azure indexing result is still required before merge.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 6f43c50f37f9.

Label changes

Label changes:

  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.

Label justifications:

  • P2: This fixes a bounded custom-provider memory-indexing failure without evidence of a broader outage or data-loss condition.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Focused mocked tests and CI claims support request construction, but the PR still lacks redacted after-fix evidence from a real Azure memory-indexing setup; add a live run, runtime log, or request trace with secrets and private endpoint details removed. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +42, Tests +122. Total +164 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 44 2 +42
Tests 1 122 0 +122
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 166 2 +164

What I checked:

  • Current-main behavior: Current main constructs the embedding POST target by appending /embeddings to client.baseUrl and sends client.headers unchanged, so an Azure api-version supplied only as a header reaches the wrong request location. (src/plugins/openai-compatible-embedding-provider.ts:330, 6f43c50f37f9)
  • Proposed fix boundary: The PR parses the base URL only for the three Azure-compatible hostname suffixes, removes a case-insensitive api-version header, preserves other query values, and leaves an existing URL query value authoritative. (src/plugins/openai-compatible-embedding-provider.ts:143, c3f5e3b49c85)
  • Upstream API contract: Microsoft documents Azure OpenAI service versioning through the required api-version query parameter, while api-key remains an HTTP header.
  • Focused test coverage: The branch adds mocked coverage for recognized Azure host suffixes, case-insensitive header matching, existing-query precedence, query preservation, and non-Azure behavior; this is good regression coverage but is not live Azure behavior proof. (src/plugins/openai-compatible-embedding-provider.azure.test.ts:1, c3f5e3b49c85)
  • Feature provenance: The remote custom OpenAI-compatible embedding endpoint capability appears to date to the merged custom-endpoint support work, making that area the relevant owner boundary rather than a new provider API. (src/plugins/openai-compatible-embedding-provider.ts:1, da0a062fa736)
  • Proof status: The PR body reports unit tests, lint, formatting, autoreview, and CI, but contains no redacted post-fix openclaw memory index result, request trace, runtime log, or equivalent live Azure evidence. The latest ClawSweeper review made the same proof request. (c3f5e3b49c85)

Likely related people:

  • mukhtharcm: Feature-history evidence connects this contributor to the merged custom OpenAI-compatible memory endpoint work that the affected embedding client now implements. (role: introduced custom endpoint support; confidence: medium; commits: ba316a10cc62, da0a062fa736; files: src/plugins/openai-compatible-embedding-provider.ts)
  • steipete: Recent merged history and documentation work connect this contributor to the surrounding OpenAI-compatible memory embedding surface; exact line ownership for this file was not available from the read-only environment. (role: recent adjacent memory/provider contributor; confidence: low; commits: da0a062fa736; files: src/plugins/openai-compatible-embedding-provider.ts, docs/providers/openai.md)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (3 earlier review cycles)
  • reviewed 2026-07-20T14:02:40.403Z sha ea5b5bf :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-20T14:44:38.798Z sha ea5b5bf :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-21T00:27:26.852Z sha fcb5a2d :: needs real behavior proof before merge. :: none

@vyncint vyncint mentioned this pull request Jul 21, 2026
Closed
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Azure OpenAI embedding provider fails — api-version not forwarded as URL query parameter

3 participants

@vyncint @Patrick-Erichsen