fix(auth): resolve base_url_env_var via get_env_value everywhere (closes #18757)#18948
Open
Sanjays2402 wants to merge 1 commit into
Open
fix(auth): resolve base_url_env_var via get_env_value everywhere (closes #18757)#18948Sanjays2402 wants to merge 1 commit into
Sanjays2402 wants to merge 1 commit into
Conversation
Closes NousResearch#18757. resolve_api_key_provider_credentials() and friends used os.getenv() to read `base_url_env_var`, which never consults ~/.hermes/.env. Result: providers configured solely via the dotenv file (e.g. Xiaomi with a custom `token-plan-cn.xiaomimimo.com` endpoint) silently fell back to the registry default and returned 401s on auxiliary tasks. The same os.getenv() vs get_env_value() pattern was fixed previously in NousResearch#16101 and NousResearch#17434; this is the third pass for base_url_env_var. PR NousResearch#17246 attempted this fix but only patched 2 of the 6 affected spots and notably missed runtime_provider.py despite claiming to fix it. This commit closes the remaining four: * hermes_cli/auth.py - get_api_key_provider_status (api_key path) [PR NousResearch#17246 hit] - get_external_process_provider_status (Copilot ACP) [missed] - resolve_api_key_provider_credentials [PR NousResearch#17246 hit] - resolve_external_process_provider_credentials [missed] * hermes_cli/runtime_provider.py - resolve_runtime_provider api_key branch [missed] * hermes_cli/model_switch.py - _refresh_curated_models built-in endpoint dedup [missed] get_env_value() preserves shell-export precedence: os.environ wins when the variable is exported, so existing deployments are unaffected. The dotenv file is consulted only as a fallback, matching how API keys have always been resolved. Adds tests/hermes_cli/test_base_url_dotenv_resolution.py covering all four fixed paths plus a guard test that shell exports still take priority.
Collaborator
1 similar comment
Collaborator
This was referenced May 4, 2026
Closed
3 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
2 participants
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.
Closes #18757
resolve_api_key_provider_credentials()and friends readbase_url_env_varviaos.getenv(), which never consults~/.hermes/.env. A user who sets, e.g.,XIAOMI_BASE_URL=https://token-plan-cn.xiaomimimo.com/v1only in the dotenv file silently falls back to the registry default and gets 401s on auxiliary tasks. API keys are read correctly viaget_env_value()— base URLs are not. This is the same bug class fixed for API keys in #16101 and for TTS/STT in #17434.Why this PR rather than #17246
#17246 (diff) targets the same issue but only patches 2 of 6 buggy spots, and despite the PR description claiming
runtime_provider.pywas "aligned", the diff doesn't touch that file at all.hermes_cli/auth.pyget_api_key_provider_statushermes_cli/auth.pyresolve_api_key_provider_credentialshermes_cli/auth.pyget_external_process_provider_status(Copilot ACP)hermes_cli/auth.pyresolve_external_process_provider_credentialshermes_cli/runtime_provider.pyresolve_runtime_provider(api_key branch)hermes_cli/model_switch.py_refresh_curated_modelsbuiltin endpoint dedup#17246 also bundles 5 unrelated fixes (cron, env precedence, moonshot detection, gateway lock, etc.) — this PR is scoped solely to #18757 so it can land without dragging the rest along.
Behavior
get_env_value()already preserves shell-export precedence —os.environwins when the variable is exported, dotenv is the fallback. Existing deployments are unaffected; users who relied on the dotenv file finally get the right endpoint.Tests
New
tests/hermes_cli/test_base_url_dotenv_resolution.py(7 tests):resolve_api_key_provider_credentials/get_api_key_provider_statusread base URL from~/.hermes/.env(Xiaomi)resolve_external_process_provider_credentials/*_statusread base URL from~/.hermes/.env(Copilot ACP)resolve_runtime_providerhonours dotenv on the api_key branchmodel_switchdedup helper is wired throughget_env_valueRun on the full intersection of test files touching the changed modules — full green, no regressions.
Files changed
hermes_cli/auth.py— top-levelget_env_valueimport + 4 call siteshermes_cli/runtime_provider.py— 1 call sitehermes_cli/model_switch.py— 1 call site (withos.environfallback for safety)tests/hermes_cli/test_base_url_dotenv_resolution.py— newCloses #18757.