fix(aux): resolve provider keys via .env-preferring lookup so rotation isn't shadowed by a stale shell export - #55972
Open
Frowtek wants to merge 1 commit into
Conversation
…n isn't shadowed by a stale shell export NousResearch#55528 established that Hermes-managed credential resolution must prefer `~/.hermes/.env` over `os.environ`: Hermes loads `.env` without overriding an existing process-env var, so a value exported in the parent shell (login profile, Codex CLI, test runner) shadows a freshly-rotated `.env` key and produces persistent 401s. That fix switched the main request path (`_resolve_api_key_provider_secret`), `get_anthropic_key`, and the Azure-Foundry status read to `get_env_value_prefer_dotenv`. The auxiliary client — which authenticates every side-LLM task (title generation, curator skill review, vision analysis, web-search summarization, session search) — still resolved provider keys via `os.getenv(...)`, so after a key rotation the main agent used the fresh key while auxiliary tasks kept authenticating with the stale shell export and 401'd. This is reachable on the common single-key setup (no credential pool → the env-var path is taken directly) and whenever the pool is exhausted. Route the auxiliary OpenRouter / OpenAI key resolution (and the OpenRouter "unavailable" diagnostic) through a small `_prefer_dotenv_env` helper that wraps `get_env_value_prefer_dotenv`, so a rotated `.env` key wins over a stale export here too. Adds a regression test asserting the auxiliary OpenRouter path resolves the rotated `.env` key over a stale `os.environ` value.
7 tasks
Collaborator
|
Thanks for extending the key-rotation fix to the auxiliary OpenRouter path. The premise is confirmed on current main: Problems
Suggested changes
Automated hermes-sweeper review. |
Open
12 tasks
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
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.
What does this PR do?
Extends the #55528 credential-rotation fix to the auxiliary client. #55528
established that Hermes-managed credential resolution must prefer
~/.hermes/.envoveros.environ: Hermes loads.envwithout overridingan existing process-env var, so a value exported in the parent shell (login
profile, Codex CLI, test runner) shadows a freshly-rotated
.envkey andproduces persistent 401s. That fix switched the main request path
(
_resolve_api_key_provider_secret),get_anthropic_key, and theAzure-Foundry status read to
get_env_value_prefer_dotenv.The auxiliary client — which authenticates every side-LLM task (title
generation, curator skill review, vision analysis, web-search summarization,
session search) — still resolved provider keys via
os.getenv(...). So after akey rotation the main agent used the fresh key while auxiliary tasks kept
authenticating with the stale shell export and 401'd — a confusing split
failure ("my chat works but titles/vision are broken"). It's reachable on the
common single-key setup (no credential pool → the env-var path is taken
directly) and whenever the pool is exhausted.
Related Issue
Sibling of #55528 / #20591 — extends the same
.env-preferring resolution tothe auxiliary client. Searched existing PRs/issues; none covers the auxiliary
path.
Type of Change
Changes Made
agent/auxiliary_client.py— add_prefer_dotenv_env(key)(wrapsget_env_value_prefer_dotenv) and route the auxiliary credential readsthrough it, so a rotated
.envkey wins over a stale shell export:_try_openrouter(OpenRouter key)_resolve_custom_runtimeandresolve_provider_client(OpenAI key)_describe_openrouter_unavailable(the "not set" diagnostic, for consistency)tests/agent/test_auxiliary_client.py— regression test asserting theauxiliary OpenRouter path resolves a rotated
.envkey over a staleos.environvalue.How to Test
Reproduction: export
OPENROUTER_API_KEY=oldin the shell, then rotate it to anew value in
~/.hermes/.env.old(stale shell export) → auxiliarytasks 401, while the main agent (fixed by fix(credentials): prefer ~/.hermes/.env over stale os.environ on key rotation (#20591) #55528) uses the new key.
.envkey, matching the main path.Tests run and results:
ruff checkandscripts/check-windows-footguns.pyon the changed files: bothclean.
Checklist
fix(aux): …)