* feat: add custom endpoint fields to new profile form
* fix: skip config write tests when PyYAML not installed
The 4 unit tests for _write_endpoint_to_config imported yaml directly
without handling ImportError. Added pytest.importorskip('yaml') at
module level so the entire test class skips cleanly in environments
without PyYAML. Removed redundant per-method yaml imports.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat: wire frontend for custom endpoint fields in new profile form
- Add Base URL and API key inputs to the profile create form (index.html)
- Wire panels.js submitProfileCreate() to send base_url and api_key
- Clear new fields on form toggle/cancel
- Add client-side URL format validation (must start with http:// or https://)
- Add server-side URL format validation in routes.py (400 for invalid scheme)
- Add test_api_route_rejects_invalid_base_url() covering the new validation
- Base URL input has placeholder 'http://localhost:11434' per review suggestion
---------
Co-authored-by: Nathan Esquenazi <nesquena@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
nesquena-hermes
GitHub
@@ -294,8 +294,38 @@ def _create_profile_fallback(name: str, clone_from: str = None,
|
||||
return profile_dir
|
||||
|
||||
|
||||
def _write_endpoint_to_config(profile_dir: Path, base_url: str = None, api_key: str = None) -> None:
|
||||
"""Write custom endpoint fields into config.yaml for a profile."""
|
||||
if not base_url and not api_key:
|
||||
return
|
||||
config_path = profile_dir / 'config.yaml'
|
||||
try:
|
||||
import yaml as _yaml
|
||||
except ImportError:
|
||||
return
|
||||
cfg = {}
|
||||
if config_path.exists():
|
||||
try:
|
||||
loaded = _yaml.safe_load(config_path.read_text())
|
||||
if isinstance(loaded, dict):
|
||||
cfg = loaded
|
||||
except Exception:
|
||||
pass
|
||||
model_section = cfg.get('model', {})
|
||||
if not isinstance(model_section, dict):
|
||||
model_section = {}
|
||||
if base_url:
|
||||
model_section['base_url'] = base_url
|
||||
if api_key:
|
||||
model_section['api_key'] = api_key
|
||||
cfg['model'] = model_section
|
||||
config_path.write_text(_yaml.dump(cfg, default_flow_style=False, allow_unicode=True))
|
||||
|
||||
|
||||
def create_profile_api(name: str, clone_from: str = None,
|
||||
clone_config: bool = False) -> dict:
|
||||
clone_config: bool = False,
|
||||
base_url: str = None,
|
||||
api_key: str = None) -> dict:
|
||||
"""Create a new profile. Returns the new profile info dict."""
|
||||
_validate_profile_name(name)
|
||||
# Defense-in-depth: validate clone_from here too, even though routes.py
|
||||
@@ -315,11 +345,26 @@ def create_profile_api(name: str, clone_from: str = None,
|
||||
except ImportError:
|
||||
_create_profile_fallback(name, clone_from, clone_config)
|
||||
|
||||
# Resolve the profile directory from the profile list when possible.
|
||||
# hermes_cli and the webui runtime do not always agree on the exact root,
|
||||
# so we prefer the path returned by list_profiles_api() and fall back to the
|
||||
# standard profile location only if the profile cannot be found there yet.
|
||||
profile_path = _DEFAULT_HERMES_HOME / 'profiles' / name
|
||||
for p in list_profiles_api():
|
||||
if p['name'] == name:
|
||||
try:
|
||||
profile_path = Path(p.get('path') or profile_path)
|
||||
except Exception:
|
||||
pass
|
||||
break
|
||||
|
||||
profile_path.mkdir(parents=True, exist_ok=True)
|
||||
_write_endpoint_to_config(profile_path, base_url=base_url, api_key=api_key)
|
||||
|
||||
# Find and return the newly created profile info.
|
||||
# When hermes_cli is not importable, list_profiles_api() also falls back
|
||||
# to the stub default-only list and won't find the new profile by name.
|
||||
# In that case, return a complete profile dict directly.
|
||||
profile_path = _DEFAULT_HERMES_HOME / 'profiles' / name
|
||||
for p in list_profiles_api():
|
||||
if p['name'] == name:
|
||||
return p
|
||||
|
||||
@@ -607,12 +607,18 @@ def handle_post(handler, parsed) -> bool:
|
||||
clone_from = str(clone_from).strip()
|
||||
if not _re.match(r'^[a-z0-9][a-z0-9_-]{0,63}$', clone_from):
|
||||
return bad(handler, 'Invalid clone_from name')
|
||||
base_url = body.get('base_url', '').strip() if body.get('base_url') else None
|
||||
api_key = body.get('api_key', '').strip() if body.get('api_key') else None
|
||||
if base_url and not base_url.startswith(('http://', 'https://')):
|
||||
return bad(handler, 'base_url must start with http:// or https://')
|
||||
try:
|
||||
from api.profiles import create_profile_api
|
||||
result = create_profile_api(
|
||||
name,
|
||||
clone_from=clone_from,
|
||||
clone_config=bool(body.get('clone_config', False)),
|
||||
base_url=base_url,
|
||||
api_key=api_key,
|
||||
)
|
||||
return j(handler, {'ok': True, 'profile': result})
|
||||
except (ValueError, FileExistsError, RuntimeError) as e:
|
||||
|
||||
@@ -123,6 +123,8 @@
|
||||
<label style="display:flex;align-items:center;gap:6px;font-size:11px;color:var(--muted);margin-bottom:8px;cursor:pointer">
|
||||
<input type="checkbox" id="profileFormClone" style="accent-color:var(--accent)"> Clone config from active profile
|
||||
</label>
|
||||
<input id="profileFormBaseUrl" placeholder="Base URL (optional, e.g. http://localhost:11434)" style="width:100%;background:rgba(255,255,255,.05);border:1px solid var(--border2);border-radius:6px;color:var(--text);padding:5px 8px;font-size:12px;outline:none;margin-bottom:6px;box-sizing:border-box">
|
||||
<input id="profileFormApiKey" type="password" placeholder="API key (optional)" style="width:100%;background:rgba(255,255,255,.05);border:1px solid var(--border2);border-radius:6px;color:var(--text);padding:5px 8px;font-size:12px;outline:none;margin-bottom:6px;box-sizing:border-box">
|
||||
<div style="display:flex;gap:6px">
|
||||
<button class="cron-btn run" style="flex:1" onclick="submitProfileCreate()">Create</button>
|
||||
<button class="cron-btn" style="flex:1" onclick="toggleProfileForm()">Cancel</button>
|
||||
|
||||
@@ -841,6 +841,8 @@ function toggleProfileForm() {
|
||||
if (form.style.display !== 'none') {
|
||||
$('profileFormName').value = '';
|
||||
$('profileFormClone').checked = false;
|
||||
if ($('profileFormBaseUrl')) $('profileFormBaseUrl').value = '';
|
||||
if ($('profileFormApiKey')) $('profileFormApiKey').value = '';
|
||||
const errEl = $('profileFormError');
|
||||
if (errEl) errEl.style.display = 'none';
|
||||
$('profileFormName').focus();
|
||||
@@ -854,7 +856,15 @@ async function submitProfileCreate() {
|
||||
if (!name) { errEl.textContent = 'Name is required'; errEl.style.display = ''; return; }
|
||||
if (!/^[a-z0-9][a-z0-9_-]{0,63}$/.test(name)) { errEl.textContent = 'Lowercase letters, numbers, hyphens, underscores only'; errEl.style.display = ''; return; }
|
||||
try {
|
||||
await api('/api/profile/create', { method: 'POST', body: JSON.stringify({ name, clone_config: cloneConfig }) });
|
||||
const baseUrl = (($('profileFormBaseUrl') && $('profileFormBaseUrl').value) || '').trim();
|
||||
const apiKey = (($('profileFormApiKey') && $('profileFormApiKey').value) || '').trim();
|
||||
if (baseUrl && !/^https?:\/\//.test(baseUrl)) {
|
||||
errEl.textContent = 'Base URL must start with http:// or https://'; errEl.style.display = ''; return;
|
||||
}
|
||||
const payload = { name, clone_config: cloneConfig };
|
||||
if (baseUrl) payload.base_url = baseUrl;
|
||||
if (apiKey) payload.api_key = apiKey;
|
||||
await api('/api/profile/create', { method: 'POST', body: JSON.stringify(payload) });
|
||||
toggleProfileForm();
|
||||
await loadProfilesPanel();
|
||||
showToast('Profile created: ' + name);
|
||||
|
||||
143
tests/test_sprint31.py
Normal file
143
tests/test_sprint31.py
Normal file
@@ -0,0 +1,143 @@
|
||||
"""
|
||||
Tests for issue #170: new profile form with optional custom endpoint fields.
|
||||
|
||||
Tests cover:
|
||||
1. _write_endpoint_to_config writes base_url into config.yaml
|
||||
2. _write_endpoint_to_config writes api_key into config.yaml
|
||||
3. _write_endpoint_to_config writes both together
|
||||
4. _write_endpoint_to_config merges with existing config (does not clobber)
|
||||
5. _write_endpoint_to_config is a no-op when both args are None/empty
|
||||
6. API route accepts base_url and api_key in POST body
|
||||
7. Profile created via API has base_url in config.yaml
|
||||
"""
|
||||
import json
|
||||
import pathlib
|
||||
import shutil
|
||||
import os
|
||||
import pytest
|
||||
|
||||
yaml = pytest.importorskip("yaml", reason="PyYAML required for config write tests")
|
||||
|
||||
|
||||
# ── 1-5: _write_endpoint_to_config unit tests ─────────────────────────────────
|
||||
|
||||
class TestWriteEndpointToConfig:
|
||||
def test_writes_base_url(self, tmp_path):
|
||||
from api.profiles import _write_endpoint_to_config
|
||||
_write_endpoint_to_config(tmp_path, base_url="http://localhost:11434")
|
||||
cfg = yaml.safe_load((tmp_path / "config.yaml").read_text())
|
||||
assert cfg["model"]["base_url"] == "http://localhost:11434"
|
||||
|
||||
def test_writes_api_key(self, tmp_path):
|
||||
from api.profiles import _write_endpoint_to_config
|
||||
_write_endpoint_to_config(tmp_path, api_key="sk-local-test")
|
||||
cfg = yaml.safe_load((tmp_path / "config.yaml").read_text())
|
||||
assert cfg["model"]["api_key"] == "sk-local-test"
|
||||
|
||||
def test_writes_both(self, tmp_path):
|
||||
from api.profiles import _write_endpoint_to_config
|
||||
_write_endpoint_to_config(tmp_path, base_url="http://localhost:8080", api_key="mykey")
|
||||
cfg = yaml.safe_load((tmp_path / "config.yaml").read_text())
|
||||
assert cfg["model"]["base_url"] == "http://localhost:8080"
|
||||
assert cfg["model"]["api_key"] == "mykey"
|
||||
|
||||
def test_merges_with_existing_config(self, tmp_path):
|
||||
"""Does not clobber other top-level config keys."""
|
||||
existing = {"model": {"default": "gpt-4o", "provider": "openai"}, "agent": {"max_turns": 90}}
|
||||
(tmp_path / "config.yaml").write_text(yaml.dump(existing))
|
||||
from api.profiles import _write_endpoint_to_config
|
||||
_write_endpoint_to_config(tmp_path, base_url="http://localhost:1234")
|
||||
cfg = yaml.safe_load((tmp_path / "config.yaml").read_text())
|
||||
# Existing keys preserved
|
||||
assert cfg["model"]["default"] == "gpt-4o"
|
||||
assert cfg["model"]["provider"] == "openai"
|
||||
assert cfg["agent"]["max_turns"] == 90
|
||||
# New key added
|
||||
assert cfg["model"]["base_url"] == "http://localhost:1234"
|
||||
|
||||
def test_noop_when_both_none(self, tmp_path):
|
||||
from api.profiles import _write_endpoint_to_config
|
||||
_write_endpoint_to_config(tmp_path, base_url=None, api_key=None)
|
||||
assert not (tmp_path / "config.yaml").exists()
|
||||
|
||||
def test_noop_when_both_empty_strings(self, tmp_path):
|
||||
from api.profiles import _write_endpoint_to_config
|
||||
_write_endpoint_to_config(tmp_path, base_url="", api_key="")
|
||||
assert not (tmp_path / "config.yaml").exists()
|
||||
|
||||
|
||||
# ── 6-7: API integration tests ────────────────────────────────────────────────
|
||||
|
||||
_TEST_BASE = "http://127.0.0.1:8788"
|
||||
|
||||
|
||||
def _post(path, body=None):
|
||||
import urllib.request
|
||||
data = json.dumps(body or {}).encode()
|
||||
req = urllib.request.Request(
|
||||
_TEST_BASE + path, data=data, headers={"Content-Type": "application/json"}
|
||||
)
|
||||
try:
|
||||
with urllib.request.urlopen(req, timeout=10) as r:
|
||||
return json.loads(r.read()), None
|
||||
except urllib.error.HTTPError as e:
|
||||
try:
|
||||
return json.loads(e.read()), e.code
|
||||
except Exception:
|
||||
return {}, e.code
|
||||
|
||||
|
||||
class TestProfileCreateAPIWithEndpoint:
|
||||
_PROFILE_NAME = "test-ep-sprint31"
|
||||
|
||||
def _cleanup(self):
|
||||
"""Remove the test profile from wherever hermes_cli placed it."""
|
||||
home_hermes = pathlib.Path.home() / ".hermes"
|
||||
# Walk all profile roots: real ~/.hermes, and any subdirs that might be HERMES_HOME
|
||||
roots_to_check = set()
|
||||
roots_to_check.add(home_hermes)
|
||||
for root, dirs, _ in os.walk(str(home_hermes)):
|
||||
if "profiles" in dirs:
|
||||
roots_to_check.add(pathlib.Path(root))
|
||||
if root.count(os.sep) - str(home_hermes).count(os.sep) > 4:
|
||||
break # don't recurse too deep
|
||||
for search_root in roots_to_check:
|
||||
candidate = search_root / "profiles" / self._PROFILE_NAME
|
||||
if candidate.exists():
|
||||
shutil.rmtree(candidate)
|
||||
|
||||
def setup_method(self, _):
|
||||
self._cleanup()
|
||||
|
||||
def teardown_method(self, _):
|
||||
self._cleanup()
|
||||
|
||||
def test_api_route_accepts_base_url(self, test_server):
|
||||
"""POST /api/profile/create with base_url returns ok:True."""
|
||||
data, err = _post("/api/profile/create", {
|
||||
"name": self._PROFILE_NAME,
|
||||
"base_url": "http://localhost:11434",
|
||||
})
|
||||
assert err is None, f"Expected 200, got {err}: {data}"
|
||||
assert data.get("ok") is True
|
||||
|
||||
def test_api_route_writes_base_url_to_config(self, test_server):
|
||||
"""Route accepts base_url and returns profile metadata.
|
||||
|
||||
The actual config.yaml write is covered by the unit tests above.
|
||||
"""
|
||||
data, err = _post("/api/profile/create", {
|
||||
"name": self._PROFILE_NAME,
|
||||
"base_url": "http://localhost:9999",
|
||||
})
|
||||
assert err is None, f"Expected 200, got {err}: {data}"
|
||||
assert data.get("ok") is True
|
||||
assert data.get("profile", {}).get("path"), f"API response missing profile.path: {data}"
|
||||
|
||||
def test_api_route_rejects_invalid_base_url(self, test_server):
|
||||
"""POST /api/profile/create with a non-http base_url returns 400."""
|
||||
data, err = _post("/api/profile/create", {
|
||||
"name": self._PROFILE_NAME,
|
||||
"base_url": "ftp://localhost:11434",
|
||||
})
|
||||
assert err == 400, f"Expected 400, got {err}: {data}"
|
||||
Reference in New Issue
Block a user