From da160d675fc0251ba4ca1bb48a630c6265af9c8c Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Fri, 10 Apr 2026 11:43:49 -0700 Subject: [PATCH] feat: custom endpoint fields in new profile form (fixes #170, closes #214) * 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) * 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 Co-authored-by: Claude Opus 4.6 (1M context) --- api/profiles.py | 49 +++++++++++++- api/routes.py | 6 ++ static/index.html | 2 + static/panels.js | 12 +++- tests/test_sprint31.py | 143 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 209 insertions(+), 3 deletions(-) create mode 100644 tests/test_sprint31.py diff --git a/api/profiles.py b/api/profiles.py index acf9165..e686340 100644 --- a/api/profiles.py +++ b/api/profiles.py @@ -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 diff --git a/api/routes.py b/api/routes.py index f9a5c63..78e39e8 100644 --- a/api/routes.py +++ b/api/routes.py @@ -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: diff --git a/static/index.html b/static/index.html index 7a52720..fd0d97e 100644 --- a/static/index.html +++ b/static/index.html @@ -123,6 +123,8 @@ + +
diff --git a/static/panels.js b/static/panels.js index f22aca0..82d53ea 100644 --- a/static/panels.js +++ b/static/panels.js @@ -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); diff --git a/tests/test_sprint31.py b/tests/test_sprint31.py new file mode 100644 index 0000000..fe325c8 --- /dev/null +++ b/tests/test_sprint31.py @@ -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}"