fix(agent): resolve custom-provider key_env preferring .env over stale os.environ (#67935) - #67947
PRATHAMESH75 wants to merge 1 commit into
Conversation
|
Thanks for tracing the stale-environment behavior. The auxiliary defect is live: Problems
Suggested changes
Automated hermes-sweeper review. |
6fdd7b9 to
c097d15
Compare
SummaryTwo PRs address the stale-environment credential complex through different runtime paths: #67843 rebuilds the cached primary client at conversation-turn boundaries, while #67947 applies dotenv-first key resolution to auxiliary custom-provider and task-provider paths. Their overlap is the named custom-provider key_env case, but only #67843's visible diff updates the already-running primary client described in #67821 and #67935. Related pull requests
Duplicates#67843 and #67947 overlap on dotenv-first named custom-provider key_env refresh for #67935, but they are not complete duplicates: #67843 changes cached primary-client lifecycle behavior, whereas #67947 changes auxiliary resolution paths. Suggested consolidationKeep #67843 open with a salvage path centered on its per-turn primary-client refresh and obtain contributor re-review of the current diff against the four previously blocking concerns. Keep #67947 open with a salvage path narrowed to auxiliary resolution, adding production-path tests for the changed named-custom and task-provider branches; alternatively, its auxiliary changes can be split out, while #67843 remains the broader implementation for the primary live-session cause. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I67935(["issue #67935 (open)"])
P67947["PR #67947 (open)"]
P67947 -->|best fix| I67935
class I67935 open
class P67947 open
class P67947 best
class P67947 target
click I67935 "https://github.com/NousResearch/hermes-agent/issues/67935"
click P67947 "https://github.com/NousResearch/hermes-agent/pull/67947"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 2 pull requests and 2 issues in this complex. Each diff was read against this issue; Assessment working set: 33 kB of PR diffs, 14 kB of issue/PR text, 6 kB of discussion (13 comments), 7 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
c097d15 to
549dad9
Compare
fix(agent): resolve custom-provider key_env preferring .env over stale os.environ (#67935)
|
…e os.environ (NousResearch#67935) A long-lived `hermes serve` (Desktop local gateway) snapshots os.environ at spawn. Custom providers, fallback-chain entries, and auxiliary tasks resolved their config-declared key_env via plain os.getenv(), so a key added or rotated in ~/.hermes/.env mid-session never reached the backend: the request fell through to the no-key-required placeholder and 401'd until the backend was restarted. Route all three key_env resolutions through the existing get_env_value_prefer_dotenv() helper (via a small _resolve_config_key_env wrapper) so a fresh .env value wins over a stale inherited one, matching the credential-pool seeding path already used for Hermes-managed credentials.
549dad9 to
96b8879
Compare
What does this PR do?
On a Desktop local gateway (or any long-lived
hermes serve), a customOpenAI-compatible provider configured with
key_envfails with 401 after~/.hermes/.envis edited or the key is added mid-session. A freshhermes chatworks, which pins this as a dotenv-reload / env-resolution problem, not a bad
key: the long-lived backend snapshots
os.environonce at spawn, and the customkey_env was resolved with plain
os.getenv(), so a mid-session.envchange wasnever seen. The request then fell through to the
no-key-requiredplaceholderand 401'd until the backend was restarted.
The codebase already has
get_env_value_prefer_dotenv()(inhermes_cli/config.py)built for exactly this "
.envedited mid-session → staleos.environ→ persistent401" class — and the credential-pool seeding path already uses it. Custom-provider
resolution didn't, which is the inconsistency this fixes.
Related Issue
Fixes #67935
Type of Change
Changes Made
agent/auxiliary_client.py: new_resolve_config_key_env()helper that resolvesa config-declared credential
key_envthroughget_env_value_prefer_dotenv()(fresh
.envwins over stale inheritedos.environ). Routed the threeconfig-driven
key_env→os.getenv()sites through it — all the same idiom,same bug class:
resolve_provider_client(the reportedrepro path — the
no-key-required401 site),_fallback_entry_api_key(fallback-chain entries),key_envresolution.tests/agent/test_auxiliary_key_env_prefer_dotenv.py: new regression tests —key present only in
.envresolves; a rotated.envvalue wins over a staleos.environvalue; unset → empty (caller keepsno-key-required); and thenamed-custom-provider resolution shape picks up the live
.envkey.How to Test
scripts/run_tests.sh tests/agent/test_auxiliary_key_env_prefer_dotenv.py -q→ passes.scripts/run_tests.sh tests/agent/test_auxiliary_named_custom_providers.py tests/tools/test_credential_pool_env_fallback.py tests/hermes_cli/test_runtime_provider_resolution.py -q→ 200 passed.hermes serve, add/rotateLONGCAT_API_KEYin~/.hermes/.envmid-session and send a request — the fresh key is now usedwithout restarting the backend.
Checklist
Code
Documentation & Housekeeping
cli-config.yaml.exampleif I added/changed config keys — N/ACONTRIBUTING.mdorAGENTS.md— N/ANote
Related to #67453 (same root-cause class on the Docker gateway path). This PR
scopes to the custom-provider
key_envcredential-resolution sites inagent/auxiliary_client.pythat the #67935 repro exercises.