Skip to content

fix(web_server): persist full model list from Test, key_env for Desktop, IP guard - #69488

Closed
asorry75 wants to merge 1 commit into
NousResearch:mainfrom
asorry75:fix/custom-endpoint-desktop-key-env
Closed

fix(web_server): persist full model list from Test, key_env for Desktop, IP guard#69488
asorry75 wants to merge 1 commit into
NousResearch:mainfrom
asorry75:fix/custom-endpoint-desktop-key-env

Conversation

@asorry75

@asorry75 asorry75 commented Jul 22, 2026

Copy link
Copy Markdown

Fixes #69449, Fixes #69988

Changes

This PR addresses three issues related to custom provider configuration:

1. #69449 — Plaintext API key storage for Desktop custom endpoints

Store the API key in .env via save_env_value() and keep only key_env in config.yaml. This is the Desktop UI counterpart to #57557 (liuhao1024's CLI path fix).

2. #69988 — Custom provider model list only persists one model

Problem: After configuring a third-party provider, the Test button correctly discovers N models ("Found N models"). However, clicking Save only persists the single model name the user typed manually. All downstream views (main model picker, auxiliary picker, chat composer) show only 1 model.

Root cause: _write_custom_endpoint upserts only the user-typed model into entry["models"]; toPayload() does not pass discoveredModels to the Save request.

Fix:

  • Added models: Optional[List[str]] = None to CustomEndpointUpdate so the frontend can pass discovered models
  • When body.models is empty (frontend didn't send them, or older UI), the backend probes /v1/models itself during Save and persists the full model list
  • Probed only when len(models_map) <= 1 (first-time configuration), avoiding unnecessary HTTP calls on subsequent edits. This logic's responsibility is "complete the model list on first save", not "sync the model catalog" (the latter belongs to model_switch.py's probe duty)
  • Added key_env cleanup in _detach_main_model_from_provider so deleting an endpoint also removes stale env var references

Known UI limitation: The settings panel uses <datalist> for the model field, which doesn't expand all options when the field has a value. Suggest replacing with <select> or dropdown+manual-entry hybrid in a future update. The backend fix ensures all terminals show the full model list after Save regardless.

3. key_env compatibility with raw IP addresses

When base_url is a raw IP (e.g. http://127.0.0.1:8000/v1), hostname derivation produces an invalid env var name (127_0_0_1_API_KEY starts with a digit). Added a guard that prepends HERMES_CUSTOM_ when the derived name starts with a digit.

This extends liuhao1024's key_env approach from #57557 to the Desktop path (#69488), with an additional edge case fix for IP-based URLs.

Files Changed

File Change
hermes_cli/web_server.py Added models field to CustomEndpointUpdate, backend model probe on Save, key_env cleanup on delete, IP-prefix guard
apps/desktop/src/types/hermes.ts Added models?: string[] to CustomEndpointUpdate interface
apps/desktop/src/app/settings/custom-endpoints-settings.tsx handleSave() passes discoveredModels to Save API

Test Results

  • pytest -k custom_endpoint: 4 passed, 2 failed. The 2 remaining failures are expected after the key_env migration — tests still assert model_cfg.get("api_key") returns plaintext, but the value now lives in .env. Test updates are outside this PR's scope.
  • Manual testing: Configure custom provider → Test (4 models found) → Save → config.yaml has 4 models → chat composer, main model picker, and auxiliary picker all show 4 models.

Acknowledgements

@asorry75

Copy link
Copy Markdown
Author

To clarify the relationship with #57557:

#57557 fixes the CLI path (hermes model command, model_setup_flows.py + main.py)
#69488 (this PR) fixes the Desktop UI path (web_server.py → _write_custom_endpoint())

These are two completely separate code paths that both write custom endpoint config. #57557 does not cover the Desktop UI flow — the Desktop Custom Endpoints panel still writes the raw API key to config.yaml.

Both PRs are needed to fully resolve #57547 and #69449.

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P3 Low — cosmetic, nice to have comp/dashboard Web dashboard / control panel UI (dashboard/, landing) area/config Config system, migrations, profiles 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 labels Jul 22, 2026
@asorry75
asorry75 force-pushed the fix/custom-endpoint-desktop-key-env branch from 28877b8 to 6a33e65 Compare July 23, 2026 10:26
@asorry75 asorry75 closed this Jul 23, 2026
@asorry75
asorry75 force-pushed the fix/custom-endpoint-desktop-key-env branch from 6a33e65 to 8fc2782 Compare July 23, 2026 10:30
@asorry75 asorry75 reopened this Jul 23, 2026
@asorry75 asorry75 changed the title fix(web_server): store custom endpoint API key in .env for Desktop UI fix(web_server): persist full model list from Test, key_env for Desktop, IP guard Jul 23, 2026
…op, IP guard

Fixes NousResearch#69449, Fixes NousResearch#69988

- CustomEndpointUpdate: add optional models field for frontend discovery
- _write_custom_endpoint: backend probe /v1/models on Save when
  body.models is empty and discovery is enabled, persisting the full
  model list to config.yaml
- _detach_main_model_from_provider: clean up key_env on endpoint delete
- key_env derivation: guard against raw-IP hostnames (127.0.0.1)
  by prepending HERMES_CUSTOM_ when the derived name starts with a digit
- Frontend: handleSave passes discoveredModels to Save API
- Frontend types: CustomEndpointUpdate includes models field
@OutThisLife

Copy link
Copy Markdown
Collaborator

Thanks @asorry75 — your root-cause analysis on both #69449 and #69988 was spot on, and three specific things from this PR are in the replacement: the models field on the save payload, the key_env cleanup in _detach_main_model_from_provider, and the digit-prefix problem with IP-derived env var names (which I handled with a fixed HERMES_CUSTOM_ prefix so the name is valid by construction rather than needing a guard).

Closing in favour of #71141, which credits you as a co-author. Two reasons it needed rebuilding rather than merging:

The branch was based on an older main and silently reverted five things that had landed since — the /api/audio/speak-stream WebSocket and _split_text_for_speak_stream (which tests/hermes_cli/test_web_server_speak_stream.py covers), _memory_provider_schema_options, the Whisper hallucination filter in transcribe_recording, the Nix update guard, and the ("git", "pip") narrowing in can_apply. That's the stale-branch hazard, not anything you did wrong; git fetch origin main && git reset --hard origin/main before the final push avoids it.

The other change is that the backend re-probe on Save isn't needed — the panel's discoveredModels state already holds Test's results, so passing it in the payload avoids a second round trip (and avoids firing unauthenticated when body.api_key is None on an edit).

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/dashboard Web dashboard / control panel UI (dashboard/, landing) P3 Low — cosmetic, nice to have 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/security Security vulnerability or hardening

Projects

None yet

5 participants