Skip to content

fix(dashboard): don't wipe hand-written provider fields on custom-endpoint edit - #67802

Closed
Frowtek wants to merge 1 commit into
NousResearch:mainfrom
Frowtek:fix/custom-endpoint-edit-preserves-fields
Closed

fix(dashboard): don't wipe hand-written provider fields on custom-endpoint edit#67802
Frowtek wants to merge 1 commit into
NousResearch:mainfrom
Frowtek:fix/custom-endpoint-edit-preserves-fields

Conversation

@Frowtek

@Frowtek Frowtek commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

_write_custom_endpoint builds a fresh entry dict from the request body and
assigns it over providers[endpoint_id], carrying nothing forward but api_key.

A providers.<name> block is not owned by that panel. It can carry keys the
dashboard has no field for, all of them load-bearing:

field what it does
api_mode the protocol the endpoint speaks
key_env where the credential comes from
extra_headers per-provider HTTP headers — config.py notes these "may carry credentials"
request_overrides extra body params

…plus a models map with more than the one model the panel names.

So an edit that only changes the default model destroys the rest:

BEFORE  api_mode, base_url, extra_headers, key_env, model, models,
        name, request_overrides
AFTER   base_url, discover_models, model, models, name

FIELDS DESTROYED: ['api_mode', 'extra_headers', 'key_env', 'request_overrides']

The provider is left with no credential wiring (key_env gone and no
api_key), talking the wrong protocol, missing its proxy auth header — from a UI
action that said nothing about any of that. The models map also collapses to the
one named model, dropping the others and their context_length.

Introduced with the custom-endpoint settings feature (#67759).

Related Issue

Fixes #

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_cli/web_server.py — merge onto the existing entry instead of replacing
    it, and merge the models map rather than overwriting it. Managed fields still
    win, so the edit itself still applies; a brand-new endpoint is unchanged.
    api_key keeps its previous semantics — a supplied key overwrites, an omitted
    one leaves the stored key in place (now via the merge rather than an explicit
    carry-forward branch).
  • tests/hermes_cli/test_web_server.py — hand-written fields survive an edit; the
    other models and their context lengths survive.

Testing

Both new tests fail on main; the feature's own two custom-endpoint tests
still pass, which is what shows the merge didn't change create/assignment
behaviour.

Web-server / model-switch / config / credential-lifecycle suites
baseline-compared against a clean origin/main worktree: identical 16
pre-existing failures, 627 → 629 passed.

Checklist

  • Bug is reproducible on main and covered by a failing-before/passing-after test
  • No regressions in the surrounding suite (baseline-compared against origin/main)
  • Change is scoped to the defect — no unrelated refactoring
  • Tested on Ubuntu 24.04

…point edit

_write_custom_endpoint builds a fresh entry dict from the request body and
assigns it over providers[endpoint_id], carrying nothing forward but api_key.

A providers.<name> block is not owned by that panel. It can carry keys the
dashboard has no field for, all of them load-bearing:

  api_mode          the protocol the endpoint speaks
  key_env           where the credential comes from
  extra_headers     per-provider HTTP headers (may carry credentials)
  request_overrides extra body params

and a models map with more than the one model the panel names.

So an edit that only changes the default model destroys the rest:

    BEFORE  api_mode, base_url, extra_headers, key_env, model, models,
            name, request_overrides
    AFTER   base_url, discover_models, model, models, name

    FIELDS DESTROYED: ['api_mode', 'extra_headers', 'key_env',
                       'request_overrides']

The provider is left with no credential wiring (key_env gone, no api_key),
talking the wrong protocol, missing its proxy auth header — from a UI action
that said nothing about any of that. The models map also collapses to the one
named model, dropping the others and their context_length.

Merge onto the existing entry instead of replacing it, and merge the models
map rather than overwriting it. Managed fields still win, so the edit itself
still applies; a brand-new endpoint is unchanged. api_key keeps its previous
semantics — a supplied key overwrites, an omitted one leaves the stored key
in place (now via the merge rather than an explicit carry-forward branch).
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/dashboard Web dashboard / control panel UI (dashboard/, landing) area/config Config system, migrations, profiles sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 20, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #68074 with your commit's authorship preserved in git log (rebase-merge). This PR went conflict-dirty after #67806 landed first — all three dashboard PRs inserted tests at the same anchor in test_web_server.py — so it was cherry-picked onto current main rather than merged directly. Thanks for the whole cluster, these were clean, well-tested fixes.

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) P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants