Skip to content

fix(provider): unify provider/credential resolution — config.yaml as single source, fail-closed on custom intent - #35056

Closed
banditburai wants to merge 5 commits into
NousResearch:mainfrom
banditburai:fix/custom-provider-resolution
Closed

fix(provider): unify provider/credential resolution — config.yaml as single source, fail-closed on custom intent#35056
banditburai wants to merge 5 commits into
NousResearch:mainfrom
banditburai:fix/custom-provider-resolution

Conversation

@banditburai

Copy link
Copy Markdown
Contributor

Parallel provider/credential resolution paths (CLI, gateway, ACP, auxiliary) had drifted, and OpenRouter was the terminal fallback in every one — so a configured custom/local provider was silently dropped to OpenRouter or had credentials mixed from another provider. This collapses them into one offline resolver → one ResolvedProvider value object → one alias table → one /v1 normalizer → one path-scoped api_mode detector, fail-closed on declared custom intent, with zero network during resolution. Closes 6 issues, supersedes 1 PR.

Issues fixed

Issue Symptom / root cause Fix (file:area) Proof
#3263 Model-switcher loses persisted config; no pre-fill for custom endpoints; probe errors swallowed main.py (switch + classified probe), config.py (model surface) test_probe_classification, test_model_flow_custom_probe, test_config_model_surface
#4600 Custom base_url drops /v1 on chat completions provider_resolution.normalize_base_url via runtime_provider._finalize_runtime test_provider_resolution, test_runtime_provider_resolution
#5358 Gateway + CLI ignore model.provider, fall back to OpenRouter when OPENROUTER_API_KEY set auth.resolve_provider (ambient short-circuit demoted below per-provider scan) test_provider_resolution_cross_path
#8919 Custom provider config ignored at runtime; api_base dropped; OPENAI_BASE_URL silently consulted config._normalize_model_api_base, runtime_provider, auxiliary_client (warn) test_config_model_surface, test_clear_stale_base_url, test_auxiliary_custom_runtime_delegation
#12146 Runs fall back to OpenRouter despite provider=custom (alias-table drift) provider_resolution.canonicalize_provider (single table), auth.py, models.py picker test_provider_resolution (alias parity), test_auxiliary_named_custom_providers
#13489 ACP provider=custom resolves the wrong credential acp_adapter/session.py threads explicit_base_url into resolution test_session_explicit_base_url

⚠ Behavior changes

Most users unaffected — these matter only if you relied on env-based routing or incidental keys.

  • OPENAI_BASE_URL no longer routes — ignored for routing, warned when set. [potentially breaking] → move it to model.base_url (or model.api_base) in config.yaml.
  • Ambient OPENAI_API_KEY / OPENROUTER_API_KEY short-circuit moved below the per-provider scan. [breaking for incidental-key setups] a configured vendor key (e.g. DEEPSEEK_API_KEY) now wins; force OpenRouter with provider: openrouter.
  • openai-api is skipped in auto-detect so a bare OPENAI_API_KEY still falls to the OpenRouter last-resort. [behavior change] → set provider: openai-api explicitly to use direct OpenAI.
  • Declared custom provider with no resolvable base_url AND no usable key now raises AuthError(code="custom_provider_unresolved") instead of silently hitting openrouter.ai with an empty key. [breaking — now errors] → supply a base_url or key. The established key-present → OpenRouter fallback is unchanged.
  • Bare custom/local base_url now gains exactly one /v1 (chat); anthropic endpoints never gain /v1. [low risk]

Back-compat

  • ResolvedProvider supports dict-style reads (get / [] / in) and as_dict() stays byte-compatible with the legacy runtime dict — existing consumers untouched.
  • model.api_base is a permanent accepted alias for model.base_url (folded at load).
  • All explicit provider: values (openai-api, openrouter, anthropic, vendors) remain reachable.

Security

Preserves host-gated key selection from security advisory GHSA-76xc-57q6-vm5m — env keys only sent to matching hosts; lookalike/path-spoof hosts rejected. The new path-scoped api_mode detector additionally resists query/fragment spoofing the old full-URL detector allowed.

Design

  • New leaf module hermes_cli/provider_resolution.py — pure, offline, import-light; the single source of truth.
  • ResolvedProvider frozen value object resolved once per lifecycle and carried on the agent (provenance: base_url_source / key_source).
  • Resolution memoized on args + config.yaml mtime/size + env fingerprint; only static sources cached — pool / OAuth / portal / process bypass so expiring credentials stay live.
  • Fail-closed scoped exactly to "custom intent + no base_url + no usable key" (preserves the key-present fallback).
  • api_mode detector / _parse_api_mode / VALID_API_MODES consolidated into the leaf (eliminates the 3 previously-divergent copies; locked by an identity test).

Resolves upstream:

Closes #3263
Closes #4600
Closes #5358
Closes #8919
Closes #12146
Closes #13489
Supersedes #17072 — strict superset of its LM-Studio runtime slice, without its ~20k LOC of unrelated divergence and with full test coverage.

@banditburai
banditburai force-pushed the fix/custom-provider-resolution branch from b5d5be5 to f0d1e1d Compare May 30, 2026 00:46
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/cli CLI entry point, hermes_cli/, setup wizard comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/acp Agent Communication Protocol adapter area/auth Authentication, OAuth, credential pools area/config Config system, migrations, profiles labels May 30, 2026
…ovider

Pure, offline, import-light leaf module: ResolvedProvider value object,
canonicalize_provider (single alias table), normalize_base_url (one /v1
normalizer), and the path-scoped api_mode detector / select_api_mode ladder.
Foundation for the unified resolver.
…-closed; demote ambient-key fallback (NousResearch#5358 NousResearch#4600 NousResearch#12146)

One staged resolver returns the ResolvedProvider object with provenance and
memoization (static sources only; pool/OAuth/process bypass). Routes both alias
tables through canonicalize_provider (NousResearch#12146). Moves the ambient OPENAI/OPENROUTER
short-circuit below the per-provider scan so a configured vendor key wins (NousResearch#5358).
Applies the /v1 normalizer once, gated to custom (NousResearch#4600). Fails closed with
AuthError(custom_provider_unresolved) when custom intent has no resolvable
endpoint and no usable key (preserving the key-present NousResearch#14676 fallback).
…NousResearch#8919)

Folds model.api_base into model.base_url at load (permanent accepted alias) and
warns once on unknown model.* keys, so config.yaml is the single source of truth
for the endpoint and a stale OPENAI_BASE_URL is no longer silently consulted.
…stence (NousResearch#3263); picker aliases via canonicalize_provider (NousResearch#12146)

/model classifies probe failures and warns-and-confirms instead of swallowing
them, pre-fills custom-endpoint fields, and persists {provider,base_url,api_key}
atomically. The model picker's alias table is overlaid on canonicalize_provider
so it can no longer drift from runtime resolution (NousResearch#12146).
…read ACP explicit_base_url (NousResearch#13489 NousResearch#5358)

The agent carries the resolved ResolvedProvider (with provenance) instead of
re-deriving credentials per request. The auxiliary client delegates wholesale to
the unified resolver (dropping its duplicate alias table and OPENAI_BASE_URL
routing) and reaches named custom providers whose name canonicalizes to custom.
ACP sessions thread explicit_base_url + target_model into resolution so a custom
provider's base_url is honored and a built-in re-derives its own (NousResearch#13489).
@banditburai
banditburai force-pushed the fix/custom-provider-resolution branch from f0d1e1d to d7da060 Compare May 30, 2026 15:49
@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jun 21, 2026
@teknium1

teknium1 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Thanks for this, @banditburai — genuinely impressive scope and the writeup (per-issue root-cause table, provenance-tracked ResolvedProvider, offline memoization, the api_mode consolidation) is some of the most careful design analysis we've seen on a resolution PR. It's clear you traced the historical drift accurately.

Closing it, though, because the drift it targets has since been fixed piecemeal on current main, and adopting a 3353-line ground-up rewrite of the hottest, most security-sensitive path (provider/credential resolution) to re-solve already-solved problems isn't a trade we can make here. Two of the proposed behavior changes would also regress current behavior. Details, mapped to each issue you referenced:

#4600/v1 on custom chat endpoints — Already fixed. hermes_cli/runtime_provider.py does path-scoped /v1 handling: append for OpenAI-compatible chat (if not url.endswith("/v1"): url += "/v1"), strip for Anthropic-style endpoints, and special-case opencode-zen. Not the old full-URL detector.

#5358 — config model.provider ignored, falls to OpenRouter — Already fixed. resolve_provider() in hermes_cli/auth.py reads config.yaml model.provider (tier 2) above the ambient OPENROUTER_API_KEY/OPENAI_API_KEY short-circuit (tier 3). A configured provider wins over an incidental key.

#8919 — custom provider config ignored at runtime — Already closed. main has full custom-provider resolution in runtime_provider.py, provider-gated OPENAI_BASE_URL honoring (only when the active provider matches), and stale-OPENAI_BASE_URL clearing on provider switch (_clear_stale_base_url).

#12146 — falls to OpenRouter despite provider=custom (alias drift) — Already closed. main resolves bare/aliased custom through a dedicated path (incl. #27132 alias-to-custom and non-loopback-URL rejection).

#13489 — ACP provider=custom resolves wrong credential — Already fixed. acp_adapter/session.py threads the ACP-supplied base_url into resolution ("base_url": base_url or runtime.get("base_url")) and calls resolve_runtime_provider(requested=...) with the session provider — the exact threading your PR proposed.

#3263 — model-switcher persistence + custom-endpoint prefillcmd_model in hermes_cli/main.py already handles switch persistence, a direct OpenAI-compatible base_url prompt (_prompt_direct_base_url), and config-field persistence.

Also: the "3 divergent api_mode copies" are already consolidated on main — a single _parse_api_mode + _VALID_API_MODES in runtime_provider.py.

Why the two behavior changes are regressions, not fixes:

Mechanically the branch is also ~3852 commits behind main, so the diff against current main is 3464 files / ~605k deletions — it would remove nearly everything merged in the interim. Not something we could cherry-pick as a unit even if the direction fit.

If you hit a specific resolution scenario that current main still gets wrong, please open a focused issue with the exact config/env + the wrong-vs-expected provider — a narrow fix on top of the current chain is very welcome. Appreciate the depth here.

@teknium1 teknium1 closed this Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools area/config Config system, migrations, profiles comp/acp Agent Communication Protocol adapter comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard P1 High — major feature broken, no workaround sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

3 participants