Skip to content

fix(aux): resolve key_env: prefix in auxiliary task config - #24088

Closed
DatTheMaster wants to merge 1 commit into
NousResearch:mainfrom
DatTheMaster:fix/auxiliary-key-env-resolution
Closed

fix(aux): resolve key_env: prefix in auxiliary task config#24088
DatTheMaster wants to merge 1 commit into
NousResearch:mainfrom
DatTheMaster:fix/auxiliary-key-env-resolution

Conversation

@DatTheMaster

@DatTheMaster DatTheMaster commented May 12, 2026

Copy link
Copy Markdown

Summary

_resolve_task_provider_model() reads per-task config from auxiliary.<task>.api_key but ignores the key_env and api_key_env fields — the documented way to reference an environment variable by name instead of hardcoding the key value.

Every other code path that resolves provider credentials already handles these fields:

  • resolve_provider_client()providers.<name>.key_env / api_key_env
  • agent_init.py — fallback entries key_env / api_key_env
  • chat_completion_helpers.py — fallback key_env / api_key_env
  • hermes_cli/providers.py — provider registry key_env

This adds the same resolution to _resolve_task_provider_model(), so that:

auxiliary:
  title_generation:
    provider: openrouter
    base_url: https://openrouter.ai/api/v1
    key_env: OPENROUTER_API_KEY

actually resolves the key from the environment, instead of silently ignoring it and sending requests with no auth (HTTP 401).

What changed

Updated (rebased + trimmed): Removed the cli.py and gateway/run.py env var bridge changes for 6 auxiliary tasks — those tasks read config.yaml directly via _get_auxiliary_task_config()load_config(), no env var bridge needed. Also changed from the key_env: prefix convention to the key_env / api_key_env field pattern, matching the established convention used by resolve_provider_client() and fallback entries.

  • agent/auxiliary_client.py — After reading cfg_api_key from task config, resolve key_env / api_key_env when no explicit api_key is set. 10 lines added.

Testing

  • 7 new tests in tests/agent/test_auxiliary_key_env_prefix.py:
    • key_env resolution with base_url (→ provider "custom")
    • api_key_env alias resolved identically
    • key_env without base_url (named provider resolves own creds)
    • Missing env var → api_key is None (not the literal var name)
    • Explicit api_key takes precedence over key_env
    • Empty-string env var → api_key is None
    • No key_env, no api_keyapi_key stays None
  • 172/172 existing test_auxiliary_client.py tests passing
  • 19/19 existing test_auxiliary_config_bridge.py tests passing

Relationship to upstream fixes

Two recent commits address related but different bugs:

  • d68a0ec38 — pass cfg_base_url/cfg_api_key when resolving named provider (was dropping them on return)
  • b389796ae — resolve api_key_env alias in resolve_provider_client() named custom provider path

Both operate at the provider level (inside resolve_provider_client()). This PR operates at the task config level (inside _resolve_task_provider_model()). They are complementary — this closes the remaining gap where key_env/api_key_env on an auxiliary task entry had no resolution path at all.

Closes #20139

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery area/config Config system, migrations, profiles labels May 12, 2026
_resolve_task_provider_model() reads per-task config from
auxiliary.<task>.api_key but ignores the key_env and api_key_env
fields, which are the documented way to reference an environment
variable by name instead of hardcoding the key value.

Every other code path that resolves provider credentials already
handles these fields:
- resolve_provider_client() — providers.<name>.key_env / api_key_env
- agent_init.py — fallback entries key_env / api_key_env
- chat_completion_helpers.py — fallback key_env / api_key_env
- hermes_cli/providers.py — provider registry key_env

This adds the same resolution to _resolve_task_provider_model(), so
that:

  auxiliary:
    title_generation:
      provider: openrouter
      base_url: https://openrouter.ai/api/v1
      key_env: OPENROUTER_API_KEY

actually resolves the key from the environment, instead of silently
ignoring it and sending requests with no auth (HTTP 401).

Precedence matches the provider-level convention: an explicit api_key
value always wins over key_env. The api_key_env snake_case alias is
also accepted, matching resolve_provider_client() behavior.

Updated from original PR: removed the env var bridge changes in
cli.py and gateway/run.py (those 6 tasks read config.yaml directly
via _get_auxiliary_task_config(), no AUXILIARY_* env var bridge
needed). Changed from key_env: prefix convention to key_env /
api_key_env field pattern, matching the established convention used
by resolve_provider_client() and fallback entries.

7 tests covering: key_env resolution with/without base_url,
api_key_env alias, missing env var, explicit api_key precedence,
empty env var, and no-key-env baseline.

Closes NousResearch#20139
@DatTheMaster
DatTheMaster force-pushed the fix/auxiliary-key-env-resolution branch from 8b72267 to 38dd8c6 Compare May 19, 2026 15:49
@DatTheMaster

Copy link
Copy Markdown
Author

Rebased onto current main and trimmed the PR:\n\n- Dropped the cli.py/gateway/run.py env var bridge changes — those 6 auxiliary tasks (compression, session_search, skills_hub, mcp, title_generation, curator) read config.yaml directly via _get_auxiliary_task_config(), so the AUXILIARY_* env var bridge is unnecessary. (The bridge is only consumed by legacy paths in vision_tools.py and browser_tool.py for vision and web_extract.)\n- Changed from the key_env: prefix convention to the key_env / api_key_env field pattern, matching the established convention already used by resolve_provider_client() and fallback entries in agent_init.py.\n- 10 lines added to auxiliary_client.py (was 221 across 4 files).\n- 7 tests (was 4) — covers key_env field resolution, api_key_env alias, precedence, missing/empty env vars, and baseline.\n- All 172 existing test_auxiliary_client.py + 19 test_auxiliary_config_bridge.py tests passing.\n\nPR #28766 was opened as a cleaner version but was marked duplicate of this one, so closing that and keeping the fix here.

@DatTheMaster

Copy link
Copy Markdown
Author

Superseded by #36080

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants