mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-25 11:10:18 +00:00
f8007d43f3
stage-257 batch (PRs #1402 + #1415): Opus pre-release advisor caught 4 issues in stage-257: 1. MUST-FIX (security): api/oauth.py::_write_auth_json — tmp.replace() preserves the temp file umask (0644 default), so OAuth access/refresh tokens landed world-readable on shared systems. Fix: tmp.chmod(0o600) BEFORE rename, with try/except OSError that warns but does not abort. 2. SHOULD-FIX: _handle_cron_history and _handle_cron_run_detail accepted job_id as a path component without validation. Mirrors the rollback path-traversal vector caught in v0.50.255 (#1405). Path() / .. does NOT normalize. New regex ^[A-Za-z0-9_-][A-Za-z0-9_.-]{0,63}$ with explicit . / .. rejection. 3. SHOULD-FIX: _handle_cron_history int(offset)/int(limit) raised ValueError on malformed input → confusing 500. Now try/except + clamp to (max(0, offset), max(1, min(500, limit))). 4. NIT: same regex applied to _handle_cron_run_detail (defense-in-depth even though path-resolve check would catch it downstream). PR #1415 follow-up: 8 pre-existing tests in test_issue1106 and test_custom_provider_display_name asserted bare model IDs but #1415 changes named-custom-provider IDs to @custom:NAME:model form when active provider differs. Tests updated to use _strip_at_prefix helper to keep checking the same invariant in the new shape. 4 regression tests in test_v050257_opus_followups.py + 8 fixed pre-existing tests. Full suite: 3602 passed, 0 failed.
181 lines
7.7 KiB
Python
181 lines
7.7 KiB
Python
"""
|
|
Tests for named custom provider display in the model dropdown (issue #557).
|
|
|
|
When a custom_providers entry carries a `name` field (e.g. "Agent37"), the
|
|
web UI model picker should show that name as the group header rather than the
|
|
generic "Custom" label.
|
|
"""
|
|
import pytest
|
|
import api.config as config
|
|
|
|
|
|
def _strip_at_prefix(model_id):
|
|
"""Strip the optional ``@provider:`` (or ``@provider:subname:``) prefix.
|
|
|
|
PR #1415 introduced provider-qualified IDs (``@custom:NAME:model``) for
|
|
named custom providers when the active provider differs. The bare-ID
|
|
assertions in this test module pre-date that change.
|
|
"""
|
|
s = str(model_id or "")
|
|
if s.startswith("@") and ":" in s:
|
|
return s.rsplit(":", 1)[1]
|
|
return s
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _isolate_models_cache():
|
|
"""Invalidate the models TTL cache before and after every test in this file."""
|
|
try:
|
|
config.invalidate_models_cache()
|
|
except Exception:
|
|
pass
|
|
yield
|
|
try:
|
|
config.invalidate_models_cache()
|
|
except Exception:
|
|
pass
|
|
|
|
|
|
def _models_with_cfg(model_cfg=None, custom_providers=None, active_provider=None):
|
|
"""Temporarily patch config.cfg, call get_available_models(), restore.
|
|
|
|
Also pins _cfg_mtime to the current config.yaml mtime before calling
|
|
get_available_models(). Without this, if a prior test wrote config.yaml
|
|
(changing its mtime), the mtime-guard inside get_available_models() fires
|
|
reload_config() which overwrites config.cfg with the real on-disk values,
|
|
silently discarding the patch and causing ordering-dependent failures.
|
|
This matches the pattern used in test_model_resolver.py.
|
|
"""
|
|
old_cfg = dict(config.cfg)
|
|
old_mtime = config._cfg_mtime
|
|
config.cfg.clear()
|
|
if model_cfg:
|
|
config.cfg["model"] = model_cfg
|
|
if custom_providers is not None:
|
|
config.cfg["custom_providers"] = custom_providers
|
|
# Pin mtime so get_available_models() skips its reload_config() guard.
|
|
try:
|
|
config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime
|
|
except Exception:
|
|
config._cfg_mtime = 0.0 # no config.yaml present; reload guard is a no-op
|
|
try:
|
|
return config.get_available_models()
|
|
finally:
|
|
config.cfg.clear()
|
|
config.cfg.update(old_cfg)
|
|
config._cfg_mtime = old_mtime
|
|
|
|
|
|
# ── Named provider shows its name in the dropdown ─────────────────────────────
|
|
|
|
class TestNamedCustomProviderGroup:
|
|
|
|
def test_named_provider_uses_name_as_group_header(self):
|
|
"""A custom_provider entry with name='Agent37' should produce
|
|
a group whose 'provider' key is 'Agent37', not 'Custom'."""
|
|
result = _models_with_cfg(
|
|
model_cfg={"provider": "custom", "base_url": "https://agent37.example.com/v1"},
|
|
custom_providers=[
|
|
{"name": "Agent37", "model": "default", "base_url": "https://agent37.example.com/v1"}
|
|
],
|
|
)
|
|
group_names = [g["provider"] for g in result.get("groups", [])]
|
|
assert "Agent37" in group_names, (
|
|
f"Expected 'Agent37' in group names, got {group_names}"
|
|
)
|
|
|
|
def test_named_provider_does_not_produce_generic_custom(self):
|
|
"""When all custom_provider entries have names, no group called 'Custom'
|
|
should appear alongside them."""
|
|
result = _models_with_cfg(
|
|
model_cfg={"provider": "custom", "base_url": "https://agent37.example.com/v1"},
|
|
custom_providers=[
|
|
{"name": "Agent37", "model": "default", "base_url": "https://agent37.example.com/v1"}
|
|
],
|
|
)
|
|
group_names = [g["provider"] for g in result.get("groups", [])]
|
|
assert "Custom" not in group_names, (
|
|
f"Expected no generic 'Custom' group when all entries are named, got {group_names}"
|
|
)
|
|
|
|
def test_named_provider_model_appears_in_its_group(self):
|
|
"""The model ID from the named entry should be inside the named group."""
|
|
result = _models_with_cfg(
|
|
model_cfg={"provider": "custom"},
|
|
custom_providers=[
|
|
{"name": "Agent37", "model": "my-llm", "base_url": "https://agent37.example.com/v1"}
|
|
],
|
|
)
|
|
agent37_group = next(
|
|
(g for g in result.get("groups", []) if g["provider"] == "Agent37"), None
|
|
)
|
|
assert agent37_group is not None, "Expected an 'Agent37' group"
|
|
# PR #1415 prefixes IDs with @custom:NAME: when active provider differs from named slug
|
|
model_ids = [_strip_at_prefix(m["id"]) for m in agent37_group.get("models", [])]
|
|
assert "my-llm" in model_ids, (
|
|
f"Expected 'my-llm' in Agent37 group models, got {model_ids}"
|
|
)
|
|
|
|
def test_multiple_named_providers_each_get_their_own_group(self):
|
|
"""Two named custom providers should produce two distinct groups."""
|
|
result = _models_with_cfg(
|
|
model_cfg={"provider": "custom"},
|
|
custom_providers=[
|
|
{"name": "Agent37", "model": "fast-model"},
|
|
{"name": "PrivateProxy", "model": "private-llm"},
|
|
],
|
|
)
|
|
group_names = [g["provider"] for g in result.get("groups", [])]
|
|
assert "Agent37" in group_names, f"Expected 'Agent37' group, got {group_names}"
|
|
assert "PrivateProxy" in group_names, f"Expected 'PrivateProxy' group, got {group_names}"
|
|
assert "Custom" not in group_names, f"No generic 'Custom' group expected, got {group_names}"
|
|
|
|
def test_multiple_models_in_same_named_provider(self):
|
|
"""Multiple entries with the same name should be collapsed into one group."""
|
|
result = _models_with_cfg(
|
|
model_cfg={"provider": "custom"},
|
|
custom_providers=[
|
|
{"name": "Agent37", "model": "model-a"},
|
|
{"name": "Agent37", "model": "model-b"},
|
|
],
|
|
)
|
|
agent37_groups = [g for g in result.get("groups", []) if g["provider"] == "Agent37"]
|
|
assert len(agent37_groups) == 1, (
|
|
f"Expected exactly one 'Agent37' group, got {len(agent37_groups)}"
|
|
)
|
|
# PR #1415 prefixes IDs with @custom:NAME: when active provider differs from named slug
|
|
model_ids = [_strip_at_prefix(m["id"]) for m in agent37_groups[0].get("models", [])]
|
|
assert "model-a" in model_ids
|
|
assert "model-b" in model_ids
|
|
|
|
|
|
# ── Unnamed entry still falls back to 'Custom' ─────────────────────────────────
|
|
|
|
class TestUnnamedCustomProviderFallback:
|
|
|
|
def test_unnamed_entry_still_produces_custom_group(self):
|
|
"""A custom_provider entry without a name should still show as 'Custom'."""
|
|
result = _models_with_cfg(
|
|
model_cfg={"provider": "custom"},
|
|
custom_providers=[
|
|
{"model": "unnamed-model"}
|
|
],
|
|
)
|
|
group_names = [g["provider"] for g in result.get("groups", [])]
|
|
assert "Custom" in group_names, (
|
|
f"Expected generic 'Custom' group for unnamed entry, got {group_names}"
|
|
)
|
|
|
|
def test_mixed_named_and_unnamed_entries(self):
|
|
"""Named and unnamed entries should appear in their respective groups."""
|
|
result = _models_with_cfg(
|
|
model_cfg={"provider": "custom"},
|
|
custom_providers=[
|
|
{"name": "Agent37", "model": "named-model"},
|
|
{"model": "unnamed-model"},
|
|
],
|
|
)
|
|
group_names = [g["provider"] for g in result.get("groups", [])]
|
|
assert "Agent37" in group_names, f"Expected 'Agent37' group, got {group_names}"
|
|
assert "Custom" in group_names, f"Expected 'Custom' group for unnamed entry, got {group_names}"
|