Skip to content

fix: resolve env vars in retrieval rerank config#414

Merged
rwmjhb merged 1 commit intoCortexReach:masterfrom
ChaoYang78:fix/rerank-env-resolution
Apr 3, 2026
Merged

fix: resolve env vars in retrieval rerank config#414
rwmjhb merged 1 commit intoCortexReach:masterfrom
ChaoYang78:fix/rerank-env-resolution

Conversation

@ChaoYang78
Copy link
Copy Markdown
Contributor

Summary

  • resolve env placeholders in retrieval.rerankApiKey before building rerank requests
  • also resolve retrieval.rerankEndpoint, retrieval.rerankModel, and retrieval.rerankProvider for consistency with env-based config
  • add a regression test covering env expansion in retrieval rerank config

Why

embedding.apiKey already supports ${ENV_VAR} expansion, but retrieval.rerankApiKey was passed through without explicit resolution in parsePluginConfig. In env-based OpenClaw configs this can lead to rerank auth failures or literal ${JINA_API_KEY} values being propagated.

Validation

  • patched running OpenClaw instance and verified memory pipeline remained healthy
  • node --test test/resolve-env-vars-array.test.mjs

@rwmjhb
Copy link
Copy Markdown
Collaborator

rwmjhb commented Mar 30, 2026

Review: APPROVE (rebase needed)

The fix is correct — parsePluginConfig was passing retrieval config fields through without resolving ${...} env placeholders, causing rerank auth failures in env-based deployments. The dedicated regression test confirms the fix works.

Before merge, please rebase onto main — the plugin-manifest-regression test failure (sessionMemory should stay disabled by default) is unrelated to your change and comes from base branch divergence.

Two things worth considering (not blocking):

  1. Eager resolution when reranking is disabled — the new code resolves rerankApiKey, rerankEndpoint, etc. unconditionally. If rerank: "none" but config still has rerankApiKey: "${RERANK_KEY}" and that env var is unset, resolveEnvVars will throw at parse time even though reranking is off. Consider gating resolution on retrieval.rerank === "cross-encoder".

  2. No validation for rerankProvider — a typo in the env var (e.g. RERANK_PROVIDER=jiina) silently falls through to Jina semantics. Pre-existing issue, but now slightly more reachable via env-var indirection.

Copy link
Copy Markdown
Collaborator

@AliceLJY AliceLJY left a comment

Choose a reason for hiding this comment

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

LGTM — changes are clean, on-topic, and well-tested. Approving.

@rwmjhb rwmjhb merged commit 0eb8702 into CortexReach:master Apr 3, 2026
5 of 6 checks passed
ChaoYang78 added a commit to ChaoYang78/memory-lancedb-pro that referenced this pull request Apr 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants