fix(dashboard): normalize model assignments + confirm-modal for backup import (#44237)
Two beta-reported dashboard bugs:
1. Models page: 'Use as -> Main model' on an analytics card sends
entry.provider, which falls back to the model's VENDOR prefix
(modelVendor('anthropic/claude-opus-4.6') == 'anthropic') when the
session row has no billing_provider. That persisted
provider: anthropic + default: anthropic/claude-opus-4.6 — a
vendor-prefixed OpenRouter slug on the NATIVE Anthropic provider.
New sessions then 400 against api.anthropic.com and the user reads
it as 'changing models does nothing'. Unknown vendors (moonshotai,
poolside, ...) were worse: a provider that can never resolve
credentials.
Fix: _normalize_main_model_assignment() at the single write
chokepoint — maps non-provider vendor names back to the user's
current aggregator (else openrouter), and runs the model through
normalize_model_for_provider() so the persisted name matches the
target provider's API format. Wired into both /api/model/set and
the profile-scoped _write_profile_model.
2. System page: 'Restore from backup' spawns hermes import with
stdin=DEVNULL, so the CLI's interactive 'Continue? [y/N]' overwrite
prompt hits EOF and auto-aborts whenever a config already exists
(always, when the dashboard is running). Fix: ConfirmDialog in the
dashboard owns the consent, then the endpoint passes --force so the
restore runs non-interactively.
Validated live: dashboard on a temp HERMES_HOME, repro'd both failure
modes pre-fix (vendor-slug write verified via config.yaml + tui
session.create; import 'Aborted.' in action-import.log), then verified
post-fix (normalized writes, modal -> --force -> restored marker file).
This commit is contained in:
parent
4717989c10
commit
9c16ca8790
4 changed files with 209 additions and 4 deletions
|
|
@ -723,6 +723,73 @@ class ModelAssignment(BaseModel):
|
|||
profile: Optional[str] = None
|
||||
|
||||
|
||||
def _normalize_main_model_assignment(provider: str, model: str) -> tuple[str, str]:
|
||||
"""Normalize a main-slot (provider, model) pair before persisting.
|
||||
|
||||
The Models page has two assignment paths and only one of them was safe:
|
||||
|
||||
- The "Change" picker sends a real Hermes provider slug — fine.
|
||||
- The per-card "Use as → Main model" menu sends ``entry.provider``
|
||||
from the analytics rows, falling back to the model's VENDOR prefix
|
||||
(``modelVendor("anthropic/claude-opus-4.6") == "anthropic"``) when
|
||||
the session row has no ``billing_provider`` (older sessions, NULL
|
||||
rows). That wrote ``provider: anthropic`` +
|
||||
``default: anthropic/claude-opus-4.6`` to config — a vendor-prefixed
|
||||
OpenRouter slug on the NATIVE Anthropic provider. New sessions then
|
||||
400 against api.anthropic.com ("model: anthropic/claude-opus-4.6 not
|
||||
found") and the user reads it as "changing models does nothing".
|
||||
|
||||
Two repairs, both at this single chokepoint so every caller inherits:
|
||||
|
||||
1. Vendor-name → Hermes-provider mapping: when the provider string is
|
||||
not a known Hermes provider/alias (e.g. ``moonshotai``, ``x-ai`` is
|
||||
known but ``poolside`` isn't) but the model is a vendor-prefixed
|
||||
aggregator slug, keep the user's CURRENT aggregator if they're on
|
||||
one, else fall back to openrouter.
|
||||
2. Model-format normalization for the resolved provider via
|
||||
``normalize_model_for_provider`` (e.g. ``anthropic/claude-opus-4.6``
|
||||
on native anthropic → ``claude-opus-4-6``).
|
||||
"""
|
||||
from hermes_cli.models import _KNOWN_PROVIDER_NAMES, normalize_provider
|
||||
from hermes_cli.model_normalize import normalize_model_for_provider
|
||||
|
||||
prov_in = (provider or "").strip()
|
||||
model_in = (model or "").strip()
|
||||
canonical = normalize_provider(prov_in)
|
||||
|
||||
if canonical not in _KNOWN_PROVIDER_NAMES and "/" in model_in:
|
||||
# Vendor prefix posing as a provider (analytics fallback). Resolve
|
||||
# against the user's current provider when it's an aggregator that
|
||||
# serves vendor-prefixed slugs; otherwise default to openrouter.
|
||||
try:
|
||||
cur_cfg = load_config().get("model", {})
|
||||
cur_provider = (
|
||||
str(cur_cfg.get("provider", "") or "").strip().lower()
|
||||
if isinstance(cur_cfg, dict) else ""
|
||||
)
|
||||
except Exception:
|
||||
cur_provider = ""
|
||||
from hermes_cli.models import _AGGREGATOR_PROVIDERS
|
||||
if cur_provider and normalize_provider(cur_provider) in _AGGREGATOR_PROVIDERS:
|
||||
canonical = normalize_provider(cur_provider)
|
||||
prov_in = cur_provider
|
||||
else:
|
||||
canonical = "openrouter"
|
||||
prov_in = "openrouter"
|
||||
|
||||
# Custom/user-config providers keep the model verbatim — the registry
|
||||
# normalizer doesn't know their namespaces.
|
||||
if canonical in _KNOWN_PROVIDER_NAMES and not canonical.startswith("custom"):
|
||||
try:
|
||||
normalized_model = normalize_model_for_provider(model_in, canonical)
|
||||
if normalized_model:
|
||||
model_in = normalized_model
|
||||
except Exception:
|
||||
_log.debug("model normalization failed for %s/%s", prov_in, model_in, exc_info=True)
|
||||
|
||||
return prov_in, model_in
|
||||
|
||||
|
||||
def _apply_main_model_assignment(
|
||||
model_cfg: "Any", provider: str, model: str, base_url: str = ""
|
||||
) -> dict:
|
||||
|
|
@ -2892,6 +2959,7 @@ def _apply_model_assignment_sync(
|
|||
if scope == "main":
|
||||
if not provider or not model:
|
||||
raise HTTPException(status_code=400, detail="provider and model required for main")
|
||||
provider, model = _normalize_main_model_assignment(provider, model)
|
||||
model_cfg = _apply_main_model_assignment(
|
||||
cfg.get("model", {}), provider, model, base_url
|
||||
)
|
||||
|
|
@ -7302,6 +7370,14 @@ async def run_backup(body: BackupRequest):
|
|||
|
||||
class ImportRequest(BaseModel):
|
||||
archive: str
|
||||
# Pass --force to `hermes import`. The spawned action runs with
|
||||
# stdin=DEVNULL, so the CLI's interactive "Continue? [y/N]" overwrite
|
||||
# prompt hits EOF and auto-aborts ("Aborted.", exit 1) whenever the
|
||||
# target already has a config — which it always does when the dashboard
|
||||
# itself is running from it. The dashboard shows its own confirm modal
|
||||
# before calling this endpoint, then sends force=True so the restore
|
||||
# proceeds non-interactively.
|
||||
force: bool = False
|
||||
|
||||
|
||||
@app.post("/api/ops/import")
|
||||
|
|
@ -7311,8 +7387,11 @@ async def run_import(body: ImportRequest):
|
|||
raise HTTPException(status_code=400, detail="archive path is required")
|
||||
if not os.path.isfile(archive):
|
||||
raise HTTPException(status_code=404, detail=f"Archive not found: {archive}")
|
||||
args = ["import", archive]
|
||||
if body.force:
|
||||
args.append("--force")
|
||||
try:
|
||||
proc = _spawn_hermes_action(["import", archive], "import")
|
||||
proc = _spawn_hermes_action(args, "import")
|
||||
except Exception as exc:
|
||||
_log.exception("Failed to spawn import")
|
||||
raise HTTPException(status_code=500, detail=f"Failed to run import: {exc}")
|
||||
|
|
@ -8106,6 +8185,7 @@ def _write_profile_model(profile_dir: Path, provider: str, model: str) -> None:
|
|||
|
||||
token = set_hermes_home_override(str(profile_dir))
|
||||
try:
|
||||
provider, model = _normalize_main_model_assignment(provider, model)
|
||||
cfg = load_config()
|
||||
cfg["model"] = _apply_main_model_assignment(cfg.get("model", {}), provider, model)
|
||||
save_config(cfg)
|
||||
|
|
|
|||
|
|
@ -1104,6 +1104,113 @@ class TestWebServerEndpoints:
|
|||
assert confirmed.status_code == 200
|
||||
assert confirmed.json()["ok"] is True
|
||||
|
||||
def test_model_set_normalizes_vendor_slug_for_native_provider(self, monkeypatch):
|
||||
"""'Use as → Main' with an OpenRouter slug + native provider must not
|
||||
persist the vendor-prefixed slug verbatim (it 400s against the native
|
||||
API and reads as "changing models does nothing")."""
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.model_cost_guard.expensive_model_warning",
|
||||
lambda *_args, **_kwargs: None,
|
||||
)
|
||||
resp = self.client.post(
|
||||
"/api/model/set",
|
||||
json={
|
||||
"scope": "main",
|
||||
"provider": "anthropic",
|
||||
"model": "anthropic/claude-opus-4.6",
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
assert data["ok"] is True
|
||||
assert data["provider"] == "anthropic"
|
||||
# Vendor prefix stripped + dots→hyphens for the native Anthropic API.
|
||||
assert data["model"] == "claude-opus-4-6"
|
||||
|
||||
from hermes_cli.config import load_config
|
||||
cfg = load_config()
|
||||
assert cfg["model"]["provider"] == "anthropic"
|
||||
assert cfg["model"]["default"] == "claude-opus-4-6"
|
||||
|
||||
def test_model_set_maps_unknown_vendor_to_aggregator(self, monkeypatch):
|
||||
"""A bare vendor name from analytics rows (no billing_provider) is not
|
||||
a Hermes provider — keep the user's aggregator instead of writing a
|
||||
provider that can never resolve credentials."""
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.model_cost_guard.expensive_model_warning",
|
||||
lambda *_args, **_kwargs: None,
|
||||
)
|
||||
from hermes_cli.config import load_config, save_config
|
||||
cfg = load_config()
|
||||
cfg["model"] = {"provider": "openrouter", "default": "openai/gpt-5.5"}
|
||||
save_config(cfg)
|
||||
|
||||
resp = self.client.post(
|
||||
"/api/model/set",
|
||||
json={
|
||||
"scope": "main",
|
||||
"provider": "moonshotai", # vendor prefix, not a provider
|
||||
"model": "moonshotai/kimi-k2.6",
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
assert data["ok"] is True
|
||||
assert data["provider"] == "openrouter"
|
||||
assert data["model"] == "moonshotai/kimi-k2.6"
|
||||
|
||||
def test_model_set_keeps_aggregator_slug_unchanged(self, monkeypatch):
|
||||
"""The happy path (picker → openrouter + vendor/model) is untouched."""
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.model_cost_guard.expensive_model_warning",
|
||||
lambda *_args, **_kwargs: None,
|
||||
)
|
||||
resp = self.client.post(
|
||||
"/api/model/set",
|
||||
json={
|
||||
"scope": "main",
|
||||
"provider": "openrouter",
|
||||
"model": "anthropic/claude-sonnet-4.6",
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
assert data["ok"] is True
|
||||
assert data["provider"] == "openrouter"
|
||||
assert data["model"] == "anthropic/claude-sonnet-4.6"
|
||||
|
||||
def test_ops_import_passes_force_flag(self, tmp_path, monkeypatch):
|
||||
"""force=True must append --force so the spawned non-interactive
|
||||
`hermes import` doesn't auto-abort at the overwrite prompt."""
|
||||
import hermes_cli.web_server as ws
|
||||
|
||||
archive = tmp_path / "backup.zip"
|
||||
import zipfile
|
||||
with zipfile.ZipFile(archive, "w") as zf:
|
||||
zf.writestr("config.yaml", "model: {}\n")
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_spawn(subcommand, name):
|
||||
captured["args"] = subcommand
|
||||
captured["name"] = name
|
||||
from types import SimpleNamespace as NS
|
||||
return NS(pid=12345)
|
||||
|
||||
monkeypatch.setattr(ws, "_spawn_hermes_action", fake_spawn)
|
||||
|
||||
resp = self.client.post(
|
||||
"/api/ops/import", json={"archive": str(archive), "force": True},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert captured["args"] == ["import", str(archive), "--force"]
|
||||
|
||||
resp = self.client.post(
|
||||
"/api/ops/import", json={"archive": str(archive)},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert captured["args"] == ["import", str(archive)]
|
||||
|
||||
|
||||
def test_reveal_env_var(self, tmp_path):
|
||||
"""POST /api/env/reveal should return the real unredacted value."""
|
||||
|
|
|
|||
|
|
@ -985,11 +985,11 @@ export const api = {
|
|||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ output }),
|
||||
}),
|
||||
runImport: (archive: string) =>
|
||||
runImport: (archive: string, force = false) =>
|
||||
fetchJSON<ActionResponse>("/api/ops/import", {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ archive }),
|
||||
body: JSON.stringify({ archive, force }),
|
||||
}),
|
||||
getHooks: () => fetchJSON<HooksResponse>("/api/ops/hooks"),
|
||||
createHook: (body: HookCreate) =>
|
||||
|
|
|
|||
|
|
@ -170,6 +170,11 @@ export default function SystemPage() {
|
|||
const [addingCred, setAddingCred] = useState(false);
|
||||
|
||||
const [importPath, setImportPath] = useState("");
|
||||
// Restore-from-backup is destructive (overwrites the live config) and the
|
||||
// spawned `hermes import` runs non-interactively (stdin is /dev/null), so
|
||||
// its CLI "Continue? [y/N]" prompt would auto-abort. The dashboard owns the
|
||||
// consent: confirm here, then call the endpoint with force=true.
|
||||
const [importConfirmOpen, setImportConfirmOpen] = useState(false);
|
||||
|
||||
// Create-hook modal.
|
||||
const [hookModalOpen, setHookModalOpen] = useState(false);
|
||||
|
|
@ -1181,11 +1186,24 @@ export default function SystemPage() {
|
|||
disabled={!importPath.trim()}
|
||||
onClick={() => {
|
||||
if (!importPath.trim()) return;
|
||||
runOp(() => api.runImport(importPath.trim()), "Import");
|
||||
setImportConfirmOpen(true);
|
||||
}}
|
||||
>
|
||||
Import
|
||||
</Button>
|
||||
<ConfirmDialog
|
||||
open={importConfirmOpen}
|
||||
title="Restore from backup?"
|
||||
description={`This will overwrite your current Hermes configuration, skills, sessions, and data with the contents of ${importPath.trim() || "the archive"}. This cannot be undone.`}
|
||||
destructive
|
||||
confirmLabel="Restore"
|
||||
cancelLabel="Cancel"
|
||||
onCancel={() => setImportConfirmOpen(false)}
|
||||
onConfirm={() => {
|
||||
setImportConfirmOpen(false);
|
||||
runOp(() => api.runImport(importPath.trim(), true), "Import");
|
||||
}}
|
||||
/>
|
||||
</CardContent>
|
||||
</Card>
|
||||
</section>
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue