mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(model-set): persist base_url/api_key on auxiliary slot assignments
Sibling of #65254 (main-slot endpoint preservation): the auxiliary scope of POST /api/model/set dropped the request's base_url/api_key on the floor, so an aux slot pinned to a custom/local endpoint silently depended on model.base_url — and broke the moment the main slot switched away and cleared it. The aux resolver already reads auxiliary.<task>.base_url/api_key (_resolve_task_provider_model); this persists them. Desktop side: setAuxiliaryToMain / applyAuxiliaryDraft now carry the user-defined provider's api_url as base_url, mirroring applyMainModel.
This commit is contained in:
parent
07e931fcb4
commit
d4ff566232
4 changed files with 149 additions and 14 deletions
|
|
@ -316,6 +316,41 @@ describe('ModelSettings', () => {
|
|||
)
|
||||
})
|
||||
|
||||
it('carries the user-defined endpoint when an aux slot is set to a local main model', async () => {
|
||||
getGlobalModelOptions.mockResolvedValueOnce({
|
||||
providers: [
|
||||
{
|
||||
name: 'Ollama',
|
||||
slug: 'local-ollama',
|
||||
models: ['qwen3:latest'],
|
||||
authenticated: true,
|
||||
is_user_defined: true,
|
||||
api_url: 'http://localhost:11434/v1'
|
||||
}
|
||||
]
|
||||
})
|
||||
getGlobalModelInfo.mockResolvedValueOnce({ provider: 'local-ollama', model: 'qwen3:latest' })
|
||||
getAuxiliaryModels.mockResolvedValueOnce({
|
||||
main: { provider: 'local-ollama', model: 'qwen3:latest' },
|
||||
tasks: [{ task: 'vision', provider: 'auto', model: '', base_url: '' }]
|
||||
})
|
||||
|
||||
await renderModelSettings()
|
||||
|
||||
const setToMainButtons = await screen.findAllByRole('button', { name: 'Set to main' })
|
||||
fireEvent.click(setToMainButtons[0])
|
||||
|
||||
await waitFor(() =>
|
||||
expect(setModelAssignment).toHaveBeenCalledWith({
|
||||
model: 'qwen3:latest',
|
||||
provider: 'local-ollama',
|
||||
scope: 'auxiliary',
|
||||
task: 'vision',
|
||||
base_url: 'http://localhost:11434/v1'
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('warns when a main switch leaves auxiliary tasks pinned to another provider', async () => {
|
||||
setModelAssignment.mockResolvedValueOnce({
|
||||
provider: 'openrouter',
|
||||
|
|
|
|||
|
|
@ -643,6 +643,20 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
}
|
||||
}, [onMainModelChanged, refresh, selectedModel, selectedProvider, selectedProviderRow])
|
||||
|
||||
// Sibling of the applyMainModel endpoint passthrough (#65254): auxiliary
|
||||
// assignments targeting a user-defined provider must carry that provider's
|
||||
// endpoint too, or the backend pins the slot without a base_url and the
|
||||
// aux resolver falls back to the (possibly different, possibly cleared)
|
||||
// main endpoint.
|
||||
const endpointForProvider = useCallback(
|
||||
(provider: string) => {
|
||||
const row = providers.find(entry => entry.slug === provider)
|
||||
|
||||
return row?.api_url ? { base_url: row.api_url } : {}
|
||||
},
|
||||
[providers]
|
||||
)
|
||||
|
||||
const setAuxiliaryToMain = useCallback(
|
||||
async (task: string) => {
|
||||
if (!mainModel) {
|
||||
|
|
@ -653,7 +667,13 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
setError('')
|
||||
|
||||
try {
|
||||
await setModelAssignment({ model: mainModel.model, provider: mainModel.provider, scope: 'auxiliary', task })
|
||||
await setModelAssignment({
|
||||
model: mainModel.model,
|
||||
provider: mainModel.provider,
|
||||
scope: 'auxiliary',
|
||||
task,
|
||||
...endpointForProvider(mainModel.provider)
|
||||
})
|
||||
await refresh()
|
||||
} catch (err) {
|
||||
setError(err instanceof Error ? err.message : String(err))
|
||||
|
|
@ -661,7 +681,7 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
setApplying(false)
|
||||
}
|
||||
},
|
||||
[mainModel, refresh]
|
||||
[endpointForProvider, mainModel, refresh]
|
||||
)
|
||||
|
||||
const applyAuxiliaryDraft = useCallback(
|
||||
|
|
@ -674,7 +694,13 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
setError('')
|
||||
|
||||
try {
|
||||
await setModelAssignment({ model: auxDraft.model, provider: auxDraft.provider, scope: 'auxiliary', task })
|
||||
await setModelAssignment({
|
||||
model: auxDraft.model,
|
||||
provider: auxDraft.provider,
|
||||
scope: 'auxiliary',
|
||||
task,
|
||||
...endpointForProvider(auxDraft.provider)
|
||||
})
|
||||
setEditingAuxTask(null)
|
||||
await refresh()
|
||||
} catch (err) {
|
||||
|
|
@ -683,7 +709,7 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) {
|
|||
setApplying(false)
|
||||
}
|
||||
},
|
||||
[auxDraft, refresh]
|
||||
[auxDraft, endpointForProvider, refresh]
|
||||
)
|
||||
|
||||
const beginAuxiliaryEdit = useCallback(
|
||||
|
|
|
|||
|
|
@ -1392,17 +1392,18 @@ class ModelAssignment(BaseModel):
|
|||
provider: str
|
||||
model: str
|
||||
task: str = ""
|
||||
# Optional OpenAI-compatible endpoint URL. Only honored for custom/local
|
||||
# providers on the main slot — lets the GUI configure a self-hosted endpoint
|
||||
# (vLLM, llama.cpp, Ollama, …) that needs no API key. The runtime resolver
|
||||
# reads model.base_url from config (it ignores OPENAI_BASE_URL), so this is
|
||||
# the path that actually wires a local endpoint into resolution.
|
||||
# Optional OpenAI-compatible endpoint URL. Honored for custom/local
|
||||
# providers on the main slot AND on auxiliary slots — lets the GUI wire a
|
||||
# self-hosted endpoint (vLLM, llama.cpp, Ollama, …) that needs no API key.
|
||||
# The runtime resolvers read model.base_url / auxiliary.<task>.base_url
|
||||
# from config (they ignore OPENAI_BASE_URL), so this is the path that
|
||||
# actually wires a local endpoint into resolution.
|
||||
base_url: str = ""
|
||||
# Optional API key for a custom/local endpoint. Persisted to
|
||||
# ``model.api_key`` (where the runtime resolver reads it) so a self-hosted
|
||||
# endpoint that requires auth works from the GUI — mirrors the key the
|
||||
# ``hermes model`` custom flow collects. Honored only on the main slot for
|
||||
# custom/local providers.
|
||||
# ``model.api_key`` (main slot) or ``auxiliary.<task>.api_key`` (aux
|
||||
# slots) — where the runtime resolvers read it — so a self-hosted
|
||||
# endpoint that requires auth works from the GUI. Mirrors the key the
|
||||
# ``hermes model`` custom flow collects.
|
||||
api_key: str = ""
|
||||
confirm_expensive_model: bool = False
|
||||
profile: Optional[str] = None
|
||||
|
|
@ -7244,7 +7245,19 @@ def _apply_model_assignment_sync(
|
|||
new_provider = provider.strip().lower()
|
||||
slot_cfg["provider"] = provider
|
||||
slot_cfg["model"] = model
|
||||
if new_provider != prev_provider and new_provider != "custom":
|
||||
if base_url:
|
||||
# Sibling of the main-slot endpoint handling (#65254): an aux
|
||||
# assignment for a custom/local endpoint must carry its own
|
||||
# base_url, or the slot silently rebinds to whatever
|
||||
# model.base_url happens to hold — and breaks entirely once the
|
||||
# main slot switches away and clears it. The auxiliary resolver
|
||||
# already reads auxiliary.<task>.base_url/api_key
|
||||
# (_resolve_task_provider_model), so persisting them here is
|
||||
# what actually wires the endpoint in.
|
||||
slot_cfg["base_url"] = base_url
|
||||
if api_key:
|
||||
slot_cfg["api_key"] = api_key
|
||||
elif new_provider != prev_provider and new_provider != "custom":
|
||||
slot_cfg.pop("base_url", None)
|
||||
clear_model_endpoint_credentials(slot_cfg)
|
||||
aux[slot] = slot_cfg
|
||||
|
|
|
|||
|
|
@ -4616,6 +4616,67 @@ class TestWebServerEndpoints:
|
|||
assert model_cfg["provider"] == "openrouter"
|
||||
assert model_cfg.get("base_url", "") == ""
|
||||
|
||||
def test_set_model_auxiliary_persists_base_url_and_api_key(self):
|
||||
"""Aux assignments for a custom/local endpoint must persist the slot's
|
||||
own base_url/api_key (sibling of the main-slot fix, #65254). Without
|
||||
them the aux resolver falls back to model.base_url — which breaks the
|
||||
moment the main slot switches away and clears it."""
|
||||
from hermes_cli.config import load_config
|
||||
|
||||
resp = self.client.post(
|
||||
"/api/model/set",
|
||||
json={
|
||||
"scope": "auxiliary",
|
||||
"task": "vision",
|
||||
"provider": "custom",
|
||||
"model": "qwen3:latest",
|
||||
"base_url": "http://localhost:11434/v1",
|
||||
"api_key": "sk-local",
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["ok"] is True
|
||||
|
||||
slot = load_config()["auxiliary"]["vision"]
|
||||
assert slot["provider"] == "custom"
|
||||
assert slot["model"] == "qwen3:latest"
|
||||
assert slot["base_url"] == "http://localhost:11434/v1"
|
||||
assert slot["api_key"] == "sk-local"
|
||||
|
||||
def test_set_model_auxiliary_provider_switch_still_clears_stale_endpoint(self):
|
||||
"""The existing stale-endpoint scrub on provider switch is preserved
|
||||
when the new assignment carries no base_url."""
|
||||
from hermes_cli.config import load_config, save_config
|
||||
|
||||
cfg = load_config()
|
||||
cfg["auxiliary"] = {
|
||||
"vision": {
|
||||
"provider": "custom",
|
||||
"model": "qwen3:latest",
|
||||
"base_url": "http://localhost:11434/v1",
|
||||
"api_key": "sk-local",
|
||||
},
|
||||
}
|
||||
save_config(cfg)
|
||||
|
||||
resp = self.client.post(
|
||||
"/api/model/set",
|
||||
json={
|
||||
"scope": "auxiliary",
|
||||
"task": "vision",
|
||||
"provider": "openrouter",
|
||||
"model": "google/gemini-2.5-flash",
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
|
||||
slot = load_config()["auxiliary"]["vision"]
|
||||
assert slot["provider"] == "openrouter"
|
||||
# load_config deep-merges DEFAULT_CONFIG, which re-materializes the
|
||||
# keys as empty strings — assert the stale endpoint VALUES are gone.
|
||||
assert not slot.get("base_url")
|
||||
assert not slot.get("api_key")
|
||||
|
||||
def test_custom_endpoints_list_includes_direct_custom_config(self):
|
||||
"""A bare model.provider=custom config should show up in Desktop even
|
||||
before the user has materialized it under providers.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue