Skip to content

fix: restore custom model list discovery with config api key - #238

Closed
ccqqlo wants to merge 1 commit into
nesquena:masterfrom
ccqqlo:fix/custom-model-discovery-api-key
Closed

fix: restore custom model list discovery with config api key#238
ccqqlo wants to merge 1 commit into
nesquena:masterfrom
ccqqlo:fix/custom-model-discovery-api-key

Conversation

@ccqqlo

@ccqqlo ccqqlo commented Apr 11, 2026

Copy link
Copy Markdown
Contributor

Summary

  • use model.api_key / providers.<active>.api_key / providers.custom.api_key when fetching models from OpenAI-compatible custom endpoints
  • keep env vars only as fallback
  • add a regression test covering authenticated /v1/models discovery and the OpenAI-style user-agent

Problem

Custom providers configured only in config.yaml could chat normally, but the Web UI model picker called /v1/models without auth and silently fell back to showing only the default model.

Testing

  • python3 -m py_compile api/config.py tests/test_model_resolver.py
  • added regression test in tests/test_model_resolver.py

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Thanks @milofuejfuef — this directly addresses the model discovery bug that's been tripping up users with custom LM Studio / Ollama setups (see #230). The fix is in the right place and the regression test is a good addition.

A few things to check before merging:

Review notes

api/config.py — key lookup order

The change adds model.api_key / providers.<active>.api_key / providers.custom.api_key as sources for the auth header when calling /v1/models. That's correct. One thing to verify: when model.provider is custom and the user has multiple entries in custom_providers, does the resolver pick the right one based on the model name match, or does it just use the first custom entry?

If custom_providers is a list and the user has more than one local endpoint, the wrong key could be picked. Worth adding a comment or a test case that covers the multi-provider-entry case.

tests/test_model_resolver.py — coverage

The new test covers the authenticated /v1/models call and the user-agent header — both good. Consider also adding a test for the fallback path: when model.api_key is absent but providers.custom.api_key is present, does the resolver still pick it up? The current test appears to set both, so the fallback logic isn't separately exercised.

No concerns with the overall approach — reading the key from config and passing it as a Bearer token to the model list endpoint is the correct fix. The env-var fallback is kept, which maintains backward compatibility for users who set keys via environment.


Could you confirm:

  1. The key lookup order — which source wins when multiple are set?
  2. Whether the multi-entry custom_providers case is handled?

Once those are clear, this looks mergeable.

@nesquena

Copy link
Copy Markdown
Owner

Full Review: PR #238 — restore custom model list discovery with config API key

Thanks @ccqqlo! Fixes a real gap — custom providers with config-only API keys couldn't discover models.

Security Audit

The API key is read from config.yaml (model.api_keyproviders.<active>.api_keyproviders.custom.api_key → env vars). This is user-provided config, not web input. The key is sent as Authorization: Bearer to the user's own endpoint — correct and safe.

One note: The User-Agent: OpenAI/Python 1.0 header spoofing is a pragmatic workaround (some endpoints check for it), but it's technically misrepresenting the client. Functional but worth documenting.

Code Review

The key lookup priority is correct and well-ordered:

  1. model.api_key — most specific
  2. providers.<active>.api_key — provider-specific
  3. providers.custom.api_key — custom provider fallback
  4. Env vars — broadest fallback

The test is comprehensive — mocks urlopen, verifies the auth header and user-agent, checks the model appears in the correct group.

No issues found. Clean, focused fix.

Tests

555 passed, 0 failed, 42 skipped. No regressions.

Verdict

Approved. Ready to merge as-is.

nesquena-hermes pushed a commit that referenced this pull request Apr 11, 2026
get_available_models() now reads api_key from config.yaml before env vars:
  1. model.api_key
  2. providers.<active>.api_key / providers.custom.api_key
  3. env var fallbacks (HERMES_API_KEY, OPENAI_API_KEY, etc.)

Also adds OpenAI/Python User-Agent header and a regression test covering
authenticated /v1/models discovery.

Fixes users with LM Studio / Ollama custom endpoints configured in
config.yaml whose model picker silently collapsed to the default model.
nesquena-hermes added a commit that referenced this pull request Apr 11, 2026
* fix: decode HTML entities before markdown processing + zh/zh-Hant translations (#239)

Adds decode() helper in renderMd() to fix double-escaping of HTML entities
from LLM output (e.g. &lt;code&gt; becoming &amp;lt;code&amp;gt; instead
of rendering). XSS-safe: decode runs before esc(), only 5 entity patterns.

Also adds 40+ missing zh (Simplified Chinese) translation keys and a new
zh-Hant (Traditional Chinese) locale with 163 keys.

Fix applied: removed duplicate settings_label_notifications key in both
zh and zh-Hant locales.

Fixes #240

* fix: restore custom model list discovery with config api key (#238)

get_available_models() now reads api_key from config.yaml before env vars:
  1. model.api_key
  2. providers.<active>.api_key / providers.custom.api_key
  3. env var fallbacks (HERMES_API_KEY, OPENAI_API_KEY, etc.)

Also adds OpenAI/Python User-Agent header and a regression test covering
authenticated /v1/models discovery.

Fixes users with LM Studio / Ollama custom endpoints configured in
config.yaml whose model picker silently collapsed to the default model.

* feat: Docker UID/GID matching to avoid root-owned .hermes files (#237)

Adds docker_init.bash with hermeswebuitoo/hermeswebui user pattern so
container files match the host user UID/GID. Prevents .hermes volume
mounts from being owned by root when using a non-root host user.

Configure via WANTED_UID and WANTED_GID env vars (default 1000/1000).
Readme updated with setup instructions.

Fix applied: removed duplicate WANTED_GID=1000 line in docker-compose.yml
that was overriding the ${GID:-1000} variable expansion.

* security: redact credentials from API responses and fix credential file permissions (#243)

Adds response-layer credential redaction to three endpoints:
  - GET /api/session — messages[], tool_calls[], and title
  - GET /api/session/export — download also redacted
  - SSE done event — session payload in stream
  - GET /api/memory — MEMORY.md and USER.md content

Adds api/startup.py with fix_credential_permissions() at server startup.
Adds 13 tests in tests/test_security_redaction.py.

Merged with #237 container detection changes in server.py.

* fix: cancel button now interrupts agent and cleans up UI state (#244)

Wires agent.interrupt() into cancel_stream() so the backend actually
stops tool execution when the user clicks Cancel, rather than only
stopping the SSE stream while the agent keeps running.

Changes:
  - api/config.py: adds AGENT_INSTANCES dict (stream_id -> AIAgent)
  - api/streaming.py: stores agent in AGENT_INSTANCES after creation,
    checks CANCEL_FLAGS immediately after store (race condition fix),
    calls agent.interrupt() in cancel_stream(), cleans up in finally block
  - static/boot.js: removes stale setStatus(cancelling) call
  - static/messages.js: setBusy(false)/setStatus('') unconditionally on cancel

Race condition fix: after storing agent in AGENT_INSTANCES, immediately
checks if CANCEL_FLAGS[stream_id] is already set (cancel arrived during
agent init) and interrupts before starting. Check is inside the same
STREAMS_LOCK acquisition, making it atomic.

New test file: tests/test_cancel_interrupt.py with 6 unit tests.

* docs: v0.46.0 release notes, bump version, update test counts

---------

Co-authored-by: Nathan Esquenazi <nesquena@gmail.com>
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Agent review — APPROVED ✅ (merged to v0.46.0)

Reviewed full diff, PR comments, and discussion thread.

What this fixes: get_available_models() was ignoring api_key in config.yaml when fetching /v1/models from custom endpoints. Users with LM Studio / Ollama setups configured config-only (no env vars) would see the model picker collapse to just the default model.

Key lookup order verified: model.api_keyproviders.<active>.api_keyproviders.custom.api_key → env var fallbacks. Clean fallthrough with no leakage.

Security: API key is user-provided config sent to the user's own endpoint. No logging of the key value. Test correctly clears all relevant env vars with monkeypatch.delenv.

No fixes needed — clean as submitted.

Tests: 1 new regression test in test_model_resolver.py. 624 passed on stage.

Merged to master via stage branch in PR #249 (v0.46.0).

JKJameson pushed a commit to JKJameson/hermes-webui that referenced this pull request Apr 25, 2026
* fix: decode HTML entities before markdown processing + zh/zh-Hant translations (nesquena#239)

Adds decode() helper in renderMd() to fix double-escaping of HTML entities
from LLM output (e.g. &lt;code&gt; becoming &amp;lt;code&amp;gt; instead
of rendering). XSS-safe: decode runs before esc(), only 5 entity patterns.

Also adds 40+ missing zh (Simplified Chinese) translation keys and a new
zh-Hant (Traditional Chinese) locale with 163 keys.

Fix applied: removed duplicate settings_label_notifications key in both
zh and zh-Hant locales.

Fixes nesquena#240

* fix: restore custom model list discovery with config api key (nesquena#238)

get_available_models() now reads api_key from config.yaml before env vars:
  1. model.api_key
  2. providers.<active>.api_key / providers.custom.api_key
  3. env var fallbacks (HERMES_API_KEY, OPENAI_API_KEY, etc.)

Also adds OpenAI/Python User-Agent header and a regression test covering
authenticated /v1/models discovery.

Fixes users with LM Studio / Ollama custom endpoints configured in
config.yaml whose model picker silently collapsed to the default model.

* feat: Docker UID/GID matching to avoid root-owned .hermes files (nesquena#237)

Adds docker_init.bash with hermeswebuitoo/hermeswebui user pattern so
container files match the host user UID/GID. Prevents .hermes volume
mounts from being owned by root when using a non-root host user.

Configure via WANTED_UID and WANTED_GID env vars (default 1000/1000).
Readme updated with setup instructions.

Fix applied: removed duplicate WANTED_GID=1000 line in docker-compose.yml
that was overriding the ${GID:-1000} variable expansion.

* security: redact credentials from API responses and fix credential file permissions (nesquena#243)

Adds response-layer credential redaction to three endpoints:
  - GET /api/session — messages[], tool_calls[], and title
  - GET /api/session/export — download also redacted
  - SSE done event — session payload in stream
  - GET /api/memory — MEMORY.md and USER.md content

Adds api/startup.py with fix_credential_permissions() at server startup.
Adds 13 tests in tests/test_security_redaction.py.

Merged with nesquena#237 container detection changes in server.py.

* fix: cancel button now interrupts agent and cleans up UI state (nesquena#244)

Wires agent.interrupt() into cancel_stream() so the backend actually
stops tool execution when the user clicks Cancel, rather than only
stopping the SSE stream while the agent keeps running.

Changes:
  - api/config.py: adds AGENT_INSTANCES dict (stream_id -> AIAgent)
  - api/streaming.py: stores agent in AGENT_INSTANCES after creation,
    checks CANCEL_FLAGS immediately after store (race condition fix),
    calls agent.interrupt() in cancel_stream(), cleans up in finally block
  - static/boot.js: removes stale setStatus(cancelling) call
  - static/messages.js: setBusy(false)/setStatus('') unconditionally on cancel

Race condition fix: after storing agent in AGENT_INSTANCES, immediately
checks if CANCEL_FLAGS[stream_id] is already set (cancel arrived during
agent init) and interrupts before starting. Check is inside the same
STREAMS_LOCK acquisition, making it atomic.

New test file: tests/test_cancel_interrupt.py with 6 unit tests.

* docs: v0.46.0 release notes, bump version, update test counts

---------

Co-authored-by: Nathan Esquenazi <nesquena@gmail.com>
SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
* fix: decode HTML entities before markdown processing + zh/zh-Hant translations (nesquena#239)

Adds decode() helper in renderMd() to fix double-escaping of HTML entities
from LLM output (e.g. &lt;code&gt; becoming &amp;lt;code&amp;gt; instead
of rendering). XSS-safe: decode runs before esc(), only 5 entity patterns.

Also adds 40+ missing zh (Simplified Chinese) translation keys and a new
zh-Hant (Traditional Chinese) locale with 163 keys.

Fix applied: removed duplicate settings_label_notifications key in both
zh and zh-Hant locales.

Fixes nesquena#240

* fix: restore custom model list discovery with config api key (nesquena#238)

get_available_models() now reads api_key from config.yaml before env vars:
  1. model.api_key
  2. providers.<active>.api_key / providers.custom.api_key
  3. env var fallbacks (HERMES_API_KEY, OPENAI_API_KEY, etc.)

Also adds OpenAI/Python User-Agent header and a regression test covering
authenticated /v1/models discovery.

Fixes users with LM Studio / Ollama custom endpoints configured in
config.yaml whose model picker silently collapsed to the default model.

* feat: Docker UID/GID matching to avoid root-owned .hermes files (nesquena#237)

Adds docker_init.bash with hermeswebuitoo/hermeswebui user pattern so
container files match the host user UID/GID. Prevents .hermes volume
mounts from being owned by root when using a non-root host user.

Configure via WANTED_UID and WANTED_GID env vars (default 1000/1000).
Readme updated with setup instructions.

Fix applied: removed duplicate WANTED_GID=1000 line in docker-compose.yml
that was overriding the ${GID:-1000} variable expansion.

* security: redact credentials from API responses and fix credential file permissions (nesquena#243)

Adds response-layer credential redaction to three endpoints:
  - GET /api/session — messages[], tool_calls[], and title
  - GET /api/session/export — download also redacted
  - SSE done event — session payload in stream
  - GET /api/memory — MEMORY.md and USER.md content

Adds api/startup.py with fix_credential_permissions() at server startup.
Adds 13 tests in tests/test_security_redaction.py.

Merged with nesquena#237 container detection changes in server.py.

* fix: cancel button now interrupts agent and cleans up UI state (nesquena#244)

Wires agent.interrupt() into cancel_stream() so the backend actually
stops tool execution when the user clicks Cancel, rather than only
stopping the SSE stream while the agent keeps running.

Changes:
  - api/config.py: adds AGENT_INSTANCES dict (stream_id -> AIAgent)
  - api/streaming.py: stores agent in AGENT_INSTANCES after creation,
    checks CANCEL_FLAGS immediately after store (race condition fix),
    calls agent.interrupt() in cancel_stream(), cleans up in finally block
  - static/boot.js: removes stale setStatus(cancelling) call
  - static/messages.js: setBusy(false)/setStatus('') unconditionally on cancel

Race condition fix: after storing agent in AGENT_INSTANCES, immediately
checks if CANCEL_FLAGS[stream_id] is already set (cancel arrived during
agent init) and interrupts before starting. Check is inside the same
STREAMS_LOCK acquisition, making it atomic.

New test file: tests/test_cancel_interrupt.py with 6 unit tests.

* docs: v0.46.0 release notes, bump version, update test counts

---------

Co-authored-by: Nathan Esquenazi <nesquena@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants