fix: keep command-auth model discovery lazy across config and setup

This commit is contained in:
Teknium
2026-09-07 21:08:26 -07:00
parent c111ede3e5
commit 520e63661c
11 changed files with 405 additions and 432 deletions
+7 -28
View File
@@ -16,7 +16,7 @@ import logging
import subprocess
import threading
import time
from typing import Callable, Optional
from typing import Optional
logger = logging.getLogger(__name__)
@@ -116,6 +116,11 @@ class CommandTokenSource:
self._token = ""
self._expires_at: float = 0.0
@property
def cache_identity(self) -> str:
"""Stable catalog identity; token rotation must not mint on cache reads."""
return f"cmd:{self._command}"
def __call__(self) -> str:
with self._lock:
if self._token and time.monotonic() < self._expires_at:
@@ -132,33 +137,7 @@ class CommandTokenSource:
return token
def build_command_token_provider(key_cmd: str, provider_label: str = "custom") -> Optional[Callable[[], str]]:
def build_command_token_provider(key_cmd: str, provider_label: str = "custom") -> Optional[CommandTokenSource]:
"""A per-request token provider for *key_cmd*, or ``None`` when unset."""
command = str(key_cmd or "").strip()
return CommandTokenSource(command, provider_label) if command else None
def resolve_probe_token(entry: dict) -> str:
"""Mint a one-shot credential from a provider entry's ``key_cmd``, or "".
For callers needing a CONCRETE token rather than the per-request callable
``build_command_token_provider`` returns — the ``/models`` catalog probes,
which build their request by hand instead of going through a wire client.
Shares the ``CommandTokenSource`` cache with the request path, so this is
a cache read rather than a fresh sign-in.
Fail-closed: any error yields "". A helper that needs an interactive
sign-in (or is simply broken) must not take down a whole picker — the
caller degrades to the pre-existing empty-key behaviour and every other
provider still renders.
"""
if not isinstance(entry, dict):
return ""
command = str(entry.get("key_cmd", "") or "").strip()
if not command:
return ""
try:
provider = build_command_token_provider(command, str(entry.get("name", "") or "custom"))
return (provider() or "").strip() if provider is not None else ""
except Exception:
return ""
@@ -0,0 +1,2 @@
haydster7
# PR #99893 salvage
+182
View File
@@ -0,0 +1,182 @@
#!/usr/bin/env python3
"""Live key_cmd catalog A/B: local authenticated HTTP, real helper, CLI PTY.
Run with the Hermes venv Python and --repo CHECKOUT --output RECEIPT_DIR.
No user environment/config is inherited by Hermes children. Unix PTY required.
"""
import argparse
import http.server
import json
import os
from pathlib import Path
import pty
import re
import select
import shlex
import subprocess
import sys
import tempfile
import threading
import time
CATALOG = ["live-configured", "live-discovered-b", "live-discovered-c"]
TOKEN = "local-eval-token-not-a-secret"
def rows_worker():
from hermes_cli.config import load_config
from hermes_cli.model_switch_providers import (
_PickerBuild, _lap_custom_provider_rows, _lap_user_provider_rows,
)
cfg = load_config()
b = _PickerBuild(current_provider="", current_base_url="", current_model="",
max_models=None, for_picker=True, force_fresh_nous_tier=False,
probe_custom_providers=True, probe_current_custom_provider=False,
refresh=False, excluded=set(), curated={})
if cfg.get("providers"):
_lap_user_provider_rows(b, cfg["providers"])
else:
_lap_custom_provider_rows(b, cfg["custom_providers"])
print("RECEIPT_ROWS=" + json.dumps(b.results))
def cli_pty(repo, env):
master, slave = pty.openpty()
import fcntl
import struct
import termios
fcntl.ioctl(slave, termios.TIOCSWINSZ, struct.pack("HHHH", 45, 140, 0, 0))
proc = subprocess.Popen([sys.executable, "-m", "hermes_cli.main", "model"],
cwd=repo, env=env, stdin=slave, stdout=slave, stderr=slave,
start_new_session=True)
os.close(slave)
data = bytearray()
selected = False
deadline = time.monotonic() + 45
try:
while time.monotonic() < deadline:
if select.select([master], [], [], 0.2)[0]:
try:
chunk = os.read(master, 65536)
except OSError:
break
if not chunk:
break
data.extend(chunk)
text = data.decode(errors="replace")
if not selected and ("Select provider:" in text or "Choice [1-" in text):
os.write(master, b"\r")
selected = True
if selected and (re.search(r"Found \d+ model", text)
or "Could not fetch models" in text):
# Let the actual picker render before cancelling; never save a selection.
end = time.monotonic() + 0.7
while time.monotonic() < end:
if select.select([master], [], [], 0.1)[0]:
data.extend(os.read(master, 65536))
os.write(master, b"\x03")
break
if proc.poll() is not None:
break
finally:
if proc.poll() is None:
proc.terminate()
proc.wait(timeout=10)
os.close(master)
text = data.decode(errors="replace")
found = re.search(r"Found (\d+) model", text)
if not found:
raise RuntimeError("CLI failed to reach model picker:\n" + text[-12000:])
return int(found.group(1)), text
def main():
ap = argparse.ArgumentParser(description=__doc__)
ap.add_argument("--repo", type=Path, required=True)
ap.add_argument("--output", type=Path, required=True)
ap.add_argument("--expect", choices=["before", "after"])
args = ap.parse_args()
repo = args.repo.resolve()
args.output.mkdir(parents=True, exist_ok=True)
requests = []
phase = ""
class Handler(http.server.BaseHTTPRequestHandler):
def do_GET(self):
authorized = self.headers.get("Authorization") == "Bearer " + TOKEN
requests.append({"phase": phase, "path": self.path, "authorized": authorized})
body = json.dumps({"data": [{"id": m} for m in CATALOG]} if authorized
else {"error": "authorization required"}).encode()
self.send_response(200 if authorized else 401)
self.send_header("Content-Type", "application/json")
self.send_header("Content-Length", str(len(body)))
self.end_headers()
self.wfile.write(body)
def log_message(self, format, *args):
return
server = http.server.ThreadingHTTPServer(("127.0.0.1", 0), Handler)
thread = threading.Thread(target=server.serve_forever, daemon=True)
thread.start()
results = {}
try:
for schema in ("providers", "custom_providers"):
for surface in ("cli", "rows"):
phase = f"{schema}/{surface}"
with tempfile.TemporaryDirectory(prefix="hermes-keycmd-live-") as tmp:
home = Path(tmp)
state = home / ".hermes"
state.mkdir()
mint_log = home / "helper-invocations"
helper = home / "mint.py"
helper.write_text("from pathlib import Path\n"
f"with Path({str(mint_log)!r}).open('a') as f: f.write('mint\\n')\n"
f"print({TOKEN!r})\n")
entry = {"name": "Live Keycmd", "base_url": f"http://127.0.0.1:{server.server_port}/v1",
"key_cmd": f"{shlex.quote(sys.executable)} {shlex.quote(str(helper))}",
"model": CATALOG[0], "models": {CATALOG[0]: {}},
"models_discovered": True}
slug = "live-keycmd" if schema == "providers" else "custom:live-keycmd"
cfg = {"model": {"provider": slug, "default": CATALOG[0]},
schema: {slug: entry} if schema == "providers" else [entry]}
# JSON is valid YAML; no third-party harness dependencies.
(state / "config.yaml").write_text(json.dumps(cfg))
env = {"HOME": str(home), "HERMES_HOME": str(state),
"PATH": "/usr/bin:/bin", "TERM": "xterm-256color", "LANG": "C.UTF-8",
"PYTHONPATH": str(repo), "PYTHONUNBUFFERED": "1"}
if surface == "cli":
count, transcript = cli_pty(repo, env)
results[phase] = {"model_count": count}
else:
code = ("import runpy; ns=runpy.run_path(" + repr(str(Path(__file__).resolve()))
+ "); ns['rows_worker']()")
proc = subprocess.run([sys.executable, "-c", code], cwd=repo, env=env,
stdin=subprocess.DEVNULL, capture_output=True, text=True, timeout=45)
transcript = proc.stdout + proc.stderr
if proc.returncode:
raise RuntimeError(transcript)
rows = json.loads(next(s.split("=", 1)[1] for s in proc.stdout.splitlines()
if s.startswith("RECEIPT_ROWS=")))
assert len(rows) == 1, rows
results[phase] = {"model_count": rows[0]["total_models"], "models": rows[0]["models"]}
results[phase]["helper_invocations"] = len(mint_log.read_text().splitlines()) if mint_log.exists() else 0
(args.output / (phase.replace("/", "-") + ".txt")).write_text(transcript)
receipt = {"repo": str(repo), "sha": subprocess.check_output(
["git", "rev-parse", "HEAD"], cwd=repo, text=True).strip(),
"server_catalog": CATALOG, "results": results, "requests": requests}
(args.output / "receipt.json").write_text(json.dumps(receipt, indent=2) + "\n")
print(json.dumps(receipt, indent=2))
if args.expect:
expected = 1 if args.expect == "before" else len(CATALOG)
assert all(row["model_count"] == expected for row in results.values()), results
assert all(row["helper_invocations"] > 0 for row in results.values()) if args.expect == "after" else True
assert all(any(r["phase"] == p and r["authorized"] == (args.expect == "after")
for r in requests) for p in results), requests
finally:
server.shutdown()
server.server_close()
if __name__ == "__main__":
main()
+3 -2
View File
@@ -230,6 +230,7 @@ def _normalize_custom_provider_entry(
normalized[field] = value
_put("api_key", _stripped("api_key"))
_put("key_cmd", _stripped("key_cmd"))
key_env = _stripped("key_env", "api_key_env")
_put("key_env", key_env)
if key_env and entry.get("api_key_env") and not entry.get("key_env"):
@@ -253,7 +254,7 @@ def _normalize_custom_provider_entry(
for field, ok in (
("context_length", lambda v: isinstance(v, int) and v > 0),
("rate_limit_delay", lambda v: isinstance(v, (int, float)) and v >= 0),
("discover_models", lambda v: isinstance(v, bool)),
("discover_models", lambda v: isinstance(v, (bool, str))),
):
if ok(entry.get(field)):
normalized[field] = entry[field]
@@ -282,7 +283,7 @@ def _custom_provider_entry_to_provider_config(
provider_entry: Dict[str, Any] = {"api": normalized["base_url"]}
for field in (
"name", "api_key", "key_env", "models", "models_discovered", "context_length",
"name", "api_key", "key_env", "key_cmd", "models", "models_discovered", "context_length",
"rate_limit_delay", "discover_models", "extra_body", "extra_headers",
"ssl_ca_cert", "ssl_verify"):
if field in normalized:
+1
View File
@@ -758,6 +758,7 @@ def _named_custom_provider_map(cfg) -> dict[str, dict[str, str]]:
"base_url": base_url,
"api_key": entry.get("api_key", ""),
"key_env": entry.get("key_env") or entry.get("api_key_env", ""),
"key_cmd": entry.get("key_cmd", ""),
"model": model,
"models": entry.get("models", {}),
"models_discovered": entry.get("models_discovered", False),
+8 -15
View File
@@ -214,6 +214,9 @@ def _discover_named_custom_models(provider_info: dict, api_key: str, configured_
should_use_ollama_native_catalog,
)
from agent.command_token_source import build_command_token_provider, materialize_probe_api_key
source = build_command_token_provider(provider_info.get("key_cmd", ""), provider_info["name"])
api_key = materialize_probe_api_key(source if source is not None else api_key)
name, base_url = provider_info["name"], provider_info["base_url"]
api_mode = provider_info.get("api_mode", "")
provider_key = (provider_info.get("provider_key") or "").strip()
@@ -254,8 +257,10 @@ def _discover_named_custom_models(provider_info: dict, api_key: str, configured_
# _save_discovered_models_to_config. A failed save is non-fatal.
if live_models:
with contextlib.suppress(Exception):
from hermes_cli.model_switch_providers import _save_discovered_models_to_config
_save_discovered_models_to_config(base_url, live_models, api_mode=api_mode, headers=extra_headers or None)
from hermes_cli.model_switch_providers import _entry_credentials, _save_discovered_models_to_config
_save_discovered_models_to_config(
base_url, live_models, api_mode=api_mode, headers=extra_headers or None,
credential_identity=_entry_credentials(provider_info, "key_env", "api_key_env")[2])
return models, native_catalog_empty
@@ -305,20 +310,8 @@ def _model_flow_named_custom(config, provider_info):
# Resolve key from env var if api_key not set directly
if not api_key and key_env:
api_key = os.environ.get(key_env, "")
# What gets PERSISTED is derived from the statically-configured credential
# only. A key_cmd token is short-lived and must never be written back into
# config.yaml — it would be stale within the hour and would shadow the
# key_cmd that is supposed to re-mint it. Hence: before the mint below.
# Only configured credentials may be persisted, never a short-lived probe token.
config_api_key = _custom_provider_api_key_config_value(provider_info, api_key)
if not api_key:
# Command-minted credential (key_cmd) — same precedence as the request
# path and the picker: after api_key/key_env, so an explicit static key
# still wins. Without this the probe below sends no Authorization
# header, an authenticated endpoint 401s, and the flow falls back to
# the single saved model.
from agent.command_token_source import resolve_probe_token
api_key = resolve_probe_token(provider_info)
# ``discover_models: false`` (default True) uses the configured ``models:`` list
# verbatim and skips the live probe, so operators can restrict the picker to the
+36 -38
View File
@@ -13,7 +13,7 @@ import time
import threading as _threading
from dataclasses import dataclass, field
from typing import Any, List, Optional
from agent.command_token_source import resolve_probe_token
from agent.command_token_source import build_command_token_provider, materialize_probe_api_key
from hermes_cli.providers import custom_provider_aliases, custom_provider_slug, get_label
from utils import base_url_host_matches
@@ -26,7 +26,7 @@ _UNCAPPED_PICKER_PROVIDERS: frozenset[str] = frozenset({"opencode-zen", "opencod
def _save_discovered_models_to_config(
api_url: str, model_ids: list[str], *, api_mode: Optional[str] = None,
headers: Optional[dict[str, str]] = None) -> None:
headers: Optional[dict[str, str]] = None, credential_identity: str | None = None) -> None:
"""Persist a successful ``/v1/models`` probe into the matching ``custom_providers`` entry.
Matches by base_url (slash-normalised), api_mode and headers. A failed config write is
@@ -51,6 +51,8 @@ def _save_discovered_models_to_config(
continue
if headers is not None and _extra_headers_from_config(entry) != headers:
continue
if credential_identity is not None and _entry_credentials(entry, "key_env", "api_key_env")[2] != credential_identity:
continue
if not _discovered_catalog_stale(entry, model_ids):
continue
entry["models"] = {model_id: {} for model_id in model_ids}
@@ -86,17 +88,25 @@ class _NativePickerModelList(list[str]):
def _fetch_picker_live_models(
api_key: str, api_url: str, native_catalog_provider: str, preserve_native_models: bool,
api_key: Any, api_url: str, native_catalog_provider: str, preserve_native_models: bool,
headers: dict[str, str] | None = None, timeout: float = 5.0,
api_mode: str | None = None) -> list[str] | None:
api_mode: str | None = None, *, cache: bool = True) -> list[str] | None:
"""Fetch picker models with native Ollama and cached generic discovery."""
from hermes_cli.models import _get_ollama_native_headers, cached_fetch_api_models
from hermes_cli.models import _get_ollama_native_headers, cached_fetch_api_models, fetch_api_models
from hermes_cli.models_local import (
_normalize_openai_base_url,
fetch_ollama_local_models,
should_use_ollama_native_catalog,
)
if callable(api_key):
# Let the catalog cache decide whether any token or network I/O is needed.
return cached_fetch_api_models(
api_key, api_url, timeout=timeout, headers=headers, api_mode=api_mode,
fetch_models=lambda: _fetch_picker_live_models(
materialize_probe_api_key(api_key), api_url, native_catalog_provider,
preserve_native_models, headers, timeout, api_mode, cache=False))
candidate_headers = _get_ollama_native_headers(api_url, api_key=api_key)
def _drop(pred) -> None:
@@ -124,7 +134,7 @@ def _fetch_picker_live_models(
return _NativePickerModelList(native_models)
# A failed native probe is not authoritative: retry the cached generic catalog.
api_url = _normalize_openai_base_url(api_url)
generic_models = cached_fetch_api_models(
generic_models = (cached_fetch_api_models if cache else fetch_api_models)(
api_key, api_url, timeout=timeout, headers=resolved_headers, api_mode=api_mode)
return generic_models if generic_models or use_native else None
@@ -468,31 +478,14 @@ def _entry_api_mode(entry: dict) -> str | None:
return str(entry.get("api_mode") or entry.get("transport") or "").strip().lower() or None
def _entry_credentials(entry: dict, *key_env_keys: str) -> tuple[str, str, str]:
"""``(inline_api_key, key_env, identity)`` — identity is the inline key, else
``env:<VAR>``, else ``cmd:<key_cmd>``, else "".
``key_cmd`` (#86891) authenticates a provider with a SHORT-LIVED bearer
minted by a command — SSO/OIDC brokers, cloud IAM, internal auth proxies.
The request path has honoured it since it landed, but the picker resolved
probe credentials from ``api_key``/``key_env`` only, so a key_cmd provider
probed ``/v1/models`` with an EMPTY key. An authenticated endpoint answers
401, discovery returns nothing, and the row collapses to its single
configured default model — indistinguishable from an endpoint that
genuinely serves one, while inference keeps working.
The identity is keyed on the COMMAND, never the minted token: the token
rotates on every refresh, so keying on its value would change the group
fingerprint constantly and force a re-probe on every open. Two entries on
one URL with different helpers still get distinct rows.
"""
inline_api_key = str(entry.get("api_key", "") or "").strip()
def _entry_credentials(entry: dict, *key_env_keys: str) -> tuple[Any, str, str]:
"""Unminted credential, env fallback, and stable grouping identity."""
key_env = str(next((entry.get(k) for k in key_env_keys if entry.get(k)), "")).strip()
key_cmd = str(entry.get("key_cmd", "") or "").strip()
identity = inline_api_key or (
f"env:{key_env}" if key_env else (f"cmd:{key_cmd}" if key_cmd else "")
)
return inline_api_key, key_env, identity
source = build_command_token_provider(entry.get("key_cmd", ""), entry.get("name") or "custom")
if source is not None:
return source, key_env, source.cache_identity
inline_api_key = str(entry.get("api_key", "") or "").strip()
return inline_api_key, key_env, inline_api_key or (f"env:{key_env}" if key_env else "")
def _discover_flag(entry: dict):
@@ -521,7 +514,7 @@ def _group_display_name(display_name: str) -> str:
def _discover_endpoint_models(
api_key: str, api_url: str, native_catalog_provider: str, has_explicit_models: bool, *,
api_key: Any, api_url: str, native_catalog_provider: str, has_explicit_models: bool, *,
headers: dict | None, api_mode: str | None, probe_live: bool, discovery_allowed: bool,
for_picker: bool) -> tuple[list | None, bool]:
"""Return ``(models, native_catalog_empty)`` for a custom endpoint row.
@@ -538,6 +531,8 @@ def _discover_endpoint_models(
api_key, api_url, native_catalog_provider, has_explicit_models,
headers=headers, timeout=timeout, api_mode=api_mode)
is_native = isinstance(live_models, _NativePickerModelList)
if is_native and has_explicit_models:
return None, False
if live_models is not None and (live_models or not has_explicit_models or is_native):
return live_models, (is_native and not live_models)
except Exception:
@@ -548,8 +543,10 @@ def _discover_endpoint_models(
cached_models = cached_fetch_api_models(
api_key, api_url, cache_only=True, timeout=timeout, headers=headers, api_mode=api_mode,
)
if cached_models:
return cached_models, False
if has_explicit_models and isinstance(cached_models, _NativePickerModelList):
return None, False
if cached_models or isinstance(cached_models, _NativePickerModelList):
return cached_models, isinstance(cached_models, _NativePickerModelList) and not cached_models
except (ImportError, OSError, RuntimeError, TimeoutError, TypeError, ValueError, http.client.HTTPException):
pass
return None, False
@@ -689,7 +686,7 @@ class _PickerBuild:
and url_norm == self.current_base_url_norm and url_match_ok))
def discover_endpoint(
self, api_key: str, api_url: str, native_provider: str, has_explicit_models: bool, *,
self, api_key: Any, api_url: str, native_provider: str, has_explicit_models: bool, *,
headers: dict | None, api_mode: str | None, discovery_allowed: bool, is_current: bool,
) -> tuple[list | None, bool, bool]:
"""Probe policy shared by sections 3 and 4 (returns ``(models, native_empty, probed)``):
@@ -864,7 +861,7 @@ def _lap_user_provider_rows(b: _PickerBuild, user_providers: dict) -> None:
ep_groups[group_key] = {
"slug": ep_name, "name": _group_display_name(display_name), "api_url": api_url, "models": [],
"has_explicit_models": False,
"api_key": inline_api_key or _scoped_key_env(key_env) or resolve_probe_token(ep_cfg),
"api_key": inline_api_key or _scoped_key_env(key_env),
"headers": headers, "api_mode": ep_cfg.get("api_mode"),
"discovery_allowed": bool(api_url) and _discover_flag(ep_cfg), "raw_names": [], "aliases": set()}
grp = ep_groups[group_key]
@@ -944,7 +941,7 @@ def _lap_custom_provider_rows(b: _PickerBuild, custom_providers: list) -> None:
if not raw_name or not api_url:
continue
inline_api_key, key_env, cred_identity = _entry_credentials(entry, "key_env")
api_key = inline_api_key or _scoped_key_env(key_env) or resolve_probe_token(entry)
api_key = inline_api_key or _scoped_key_env(key_env)
api_mode = _entry_api_mode(entry)
discover = _discover_flag(entry)
entry_extra_headers = _extra_headers_from_config(entry)
@@ -954,7 +951,7 @@ def _lap_custom_provider_rows(b: _PickerBuild, custom_providers: list) -> None:
display_name = prefix or raw_name
grp = groups.setdefault(group_key, {
"slug": custom_provider_slug(display_name, provider_key), "name": display_name,
"api_url": api_url, "api_key": "", "models": [], "has_explicit_models": False,
"api_url": api_url, "api_key": "", "credential_identity": cred_identity, "models": [], "has_explicit_models": False,
"discover_models": True, "api_mode": api_mode, "extra_headers": entry_extra_headers,
"aliases": set()})
grp["api_key"] = grp["api_key"] or api_key # first member with a key wins
@@ -1001,7 +998,8 @@ def _lap_custom_provider_rows(b: _PickerBuild, custom_providers: list) -> None:
if probe_live: # a successful live probe persists the catalog for no-probe surfaces
try:
_save_discovered_models_to_config(
api_url, discovered, api_mode=grp.get("api_mode"), headers=grp.get("extra_headers") or None)
api_url, discovered, api_mode=grp.get("api_mode"), headers=grp.get("extra_headers") or None,
credential_identity=grp["credential_identity"])
except Exception:
pass
b.add_endpoint_row(slug, grp["name"], grp["api_url"], grp["models"], is_current, native_catalog_empty)
+31 -15
View File
@@ -2312,13 +2312,15 @@ def fetch_api_models(
def _custom_endpoint_fingerprint(
api_key: Optional[str], api_mode: Optional[str], headers: Optional[dict[str, str]]) -> str:
api_key: Any, api_mode: Optional[str], headers: Optional[dict[str, str]]) -> str:
"""Custom endpoints have no ``PROVIDER_REGISTRY`` slug, so hash exactly what callers pass to
:func:`fetch_api_models`: a rotated ``api_key``, changed ``api_mode`` or edited ``extra_headers``
each bust the cache entry. blake2b for the same CodeQL rationale as ``_credential_fingerprint``."""
import hashlib
blob = "|".join((api_key or "", api_mode or "", json.dumps(headers or {}, sort_keys=True)))
from agent.command_token_source import CommandTokenSource
identity = api_key.cache_identity if isinstance(api_key, CommandTokenSource) else api_key
blob = "|".join((identity or "", api_mode or "", json.dumps(headers or {}, sort_keys=True)))
return hashlib.blake2b(blob.encode("utf-8", errors="replace"), digest_size=8).hexdigest()
@@ -2337,15 +2339,28 @@ def _cache_entry_valid(
def cached_fetch_api_models(
api_key: Optional[str], base_url: Optional[str], *, timeout: float = 5.0,
api_key: Any, base_url: Optional[str], *, timeout: float = 5.0,
api_mode: Optional[str] = None, headers: Optional[dict[str, str]] = None,
force_refresh: bool = False, cache_only: bool = False,
fetch_models=None,
ttl_seconds: int = _PROVIDER_MODELS_CACHE_TTL) -> Optional[list[str]]:
"""Disk-cached :func:`fetch_api_models` for custom endpoints. ``cache_only`` callers (GUI picker
opens that must not block on a stopped local endpoint) still get a warm catalog instead of
collapsing to the config-declared subset."""
collapsing to the config-declared subset. ``fetch_models`` supplies native-aware discovery
without minting a command token before cache admission."""
from hermes_cli.model_switch_providers import _NativePickerModelList
def _catalog(entry):
return (_NativePickerModelList if entry.get("native_catalog") else list)(entry["models"])
def _entry(live, at=None):
return {**_cache_entry(fp, live, at), "native_catalog": isinstance(live, _NativePickerModelList)}
def _live():
return fetch_api_models(api_key, base_url, timeout=timeout, api_mode=api_mode, headers=headers)
if fetch_models is not None:
return fetch_models()
from agent.command_token_source import materialize_probe_api_key
return fetch_api_models(materialize_probe_api_key(api_key), base_url, timeout=timeout, api_mode=api_mode, headers=headers)
normalized_url = str(base_url or "").strip().rstrip("/").lower()
if not normalized_url: # nothing to key the cache on
@@ -2356,32 +2371,33 @@ def cached_fetch_api_models(
cache = _load_provider_models_cache()
entry = cache.get(cache_key)
now = time.time()
valid = not force_refresh and _cache_entry_valid(entry, fp)
valid = not force_refresh and _cache_entry_valid(entry, fp, allow_empty=isinstance(entry, dict) and entry.get("native_catalog") is True)
if cache_only:
# Same trust window as the SWR tier below, minus the revalidation.
return list(entry["models"]) if valid and now - entry["at"] < _PROVIDER_MODELS_STALE_SERVE_MAX else None
return _catalog(entry) if valid and now - entry["at"] < _PROVIDER_MODELS_STALE_SERVE_MAX else None
if valid:
age = now - entry["at"]
if age < ttl_seconds:
return list(entry["models"])
return _catalog(entry)
if age < _PROVIDER_MODELS_STALE_SERVE_MAX:
# Stale-while-revalidate: serve now, refresh off-thread for the next open.
def _refresh_custom():
live = _live()
return _cache_entry(fp, live) if live else None
return _entry(live) if live or isinstance(live, _NativePickerModelList) else None
_spawn_swr_refresh(cache_key, _refresh_custom)
return list(entry["models"])
return _catalog(entry)
live = _live()
if live:
_store_cache_entry(cache_key, _cache_entry(fp, live, now), cache)
return list(live)
if live or isinstance(live, _NativePickerModelList):
stored = _entry(live, now)
_store_cache_entry(cache_key, stored, cache)
return _catalog(stored)
# Live returned nothing (offline, timeout, auth hiccup): a stale same-fingerprint entry beats it.
if _cache_entry_valid(entry, fp):
return list(entry["models"])
if _cache_entry_valid(entry, fp, allow_empty=isinstance(entry, dict) and entry.get("native_catalog") is True):
return _catalog(entry)
return live
@@ -1602,6 +1602,7 @@ def test_model_flow_named_custom_persists_discovered_models(monkeypatch):
{
"api_mode": "anthropic_messages",
"headers": {"X-Tenant": "dragomes"},
"credential_identity": "sk-test",
},
)
], (
+125 -334
View File
@@ -1,334 +1,125 @@
"""The model picker must probe with a ``key_cmd``-minted credential.
``key_cmd`` (#86891) lets a provider authenticate with a SHORT-LIVED bearer
minted by a command — SSO/OIDC brokers, cloud IAM, internal auth proxies. The
request path has honoured it since it landed, but the picker resolved probe
credentials from ``api_key``/``key_env`` ONLY.
The failure is quiet and easy to misread. The picker probes ``/v1/models`` with
an EMPTY key, an authenticated endpoint answers 401, discovery returns nothing,
and the provider falls back to its single configured ``default_model``. The
user sees ONE model and cannot tell that apart from an endpoint that genuinely
serves one — inference itself keeps working, because that path mints correctly.
The same gap existed in the ``hermes model`` setup flow
(``_model_flow_named_custom``), which builds its own ``Authorization: Bearer``
header from the same incomplete resolution — same bug class, sibling path.
These tests pin:
* a ``key_cmd`` entry probes with the minted token, so the full catalog shows;
* the cache fingerprint is keyed on the COMMAND, not the minted token (which
rotates every refresh — keying on it would re-probe on every open);
* a broken/interactive helper degrades to the pre-existing empty-key behaviour
rather than taking the whole picker down;
* a minted token is NEVER persisted back into ``config.yaml`` — it would be
stale within the hour and would shadow the ``key_cmd`` meant to re-mint it.
"""
from __future__ import annotations
from agent.command_token_source import resolve_probe_token
class TestResolveProbeToken:
"""The shared credential helper both probe paths call."""
def test_bare_token_stdout(self):
assert resolve_probe_token(
{"key_cmd": "printf 'tok-abc'", "name": "gw"}
) == "tok-abc"
def test_json_access_token(self):
"""The OAuth 2.0 token-endpoint response shape."""
entry = {
"key_cmd": """printf '{"access_token":"tok-json","expires_in":3600}'""",
"name": "gw",
}
assert resolve_probe_token(entry) == "tok-json"
def test_absent_key_cmd_is_empty(self):
"""No key_cmd — the caller falls through to api_key/key_env."""
assert resolve_probe_token({"name": "gw"}) == ""
def test_blank_key_cmd_is_empty(self):
assert resolve_probe_token({"key_cmd": " ", "name": "gw"}) == ""
def test_failing_helper_degrades_to_empty(self):
"""A helper that needs an interactive sign-in (or is simply broken)
must not take down the picker: every other provider's row still
renders, and this one degrades to the old empty-key behaviour."""
assert resolve_probe_token({"key_cmd": "exit 1", "name": "gw"}) == ""
def test_silent_helper_is_empty(self):
assert resolve_probe_token({"key_cmd": "true", "name": "gw"}) == ""
def test_multiline_output_is_rejected(self):
"""command_token_source refuses to guess which line is the token."""
assert resolve_probe_token(
{"key_cmd": "printf 'a\\nb'", "name": "gw"}
) == ""
class TestPickerProbesWithMintedCredential:
"""End-to-end: a key_cmd provider lists its full catalog, not just one."""
def _probe_capture(self, monkeypatch):
"""Record the api_key the picker hands the live /models probe."""
seen: dict = {}
# The real callee takes keyword extras (headers, timeout, api_mode);
# the probe is wrapped in `except Exception: pass`, so a stub with a
# narrower signature would be silently swallowed and look like "the
# probe never ran".
#
# Behave like a real authenticated endpoint: no credential -> no
# catalog. A stub that returns models regardless would still pass
# unpatched (the caller falls back to default_model), so the
# model-count assertions below would prove nothing.
def fake_fetch(api_key, api_url, provider, preserve_native_models, **kwargs):
seen["api_key"] = api_key
seen["api_url"] = api_url
if not api_key:
return None # what a 401 looks like to the picker
return ["model-a", "model-b", "model-c"]
monkeypatch.setattr(
"hermes_cli.model_switch_providers._fetch_picker_live_models", fake_fetch
)
return seen
def test_probe_receives_the_minted_token(self, monkeypatch):
from hermes_cli.model_switch import list_authenticated_providers
seen = self._probe_capture(monkeypatch)
rows = list_authenticated_providers(
user_providers={
"gw": {
"base_url": "https://gw.example.test/v1",
"api_mode": "chat_completions",
"key_cmd": "printf 'tok-minted'",
"default_model": "model-a",
}
},
refresh=True,
for_picker=True,
)
assert seen.get("api_key") == "tok-minted", (
"picker probed with an empty key — a key_cmd endpoint 401s and "
"collapses to its single default_model"
)
gw = [r for r in rows if isinstance(r, dict) and r.get("slug") == "gw"]
assert gw, "key_cmd provider missing from the picker entirely"
# The user-visible symptom: unpatched this is 1 (just default_model).
assert len(gw[0].get("models") or []) == 3
def test_static_api_key_still_wins(self, monkeypatch):
"""key_cmd is a FALLBACK here: an explicit api_key is used as-is, so
this change cannot alter behaviour for existing static-key configs."""
from hermes_cli.model_switch import list_authenticated_providers
seen = self._probe_capture(monkeypatch)
list_authenticated_providers(
user_providers={
"gw": {
"base_url": "https://gw.example.test/v1",
"api_key": "sk-static",
"key_cmd": "printf 'tok-minted'",
"default_model": "model-a",
}
},
refresh=True,
for_picker=True,
)
assert seen.get("api_key") == "sk-static"
class TestCacheFingerprintStability:
"""The picker's cache key must not rotate with the token."""
def test_fingerprint_keys_on_command_not_token(self, monkeypatch):
"""A helper minting a DIFFERENT token each call must still produce a
stable cache fingerprint. Keying on the minted value would change the
fingerprint on every refresh and force a re-probe on every open."""
from hermes_cli.model_switch import list_authenticated_providers
calls = {"n": 0}
def fake_fetch(api_key, api_url, provider, preserve_native_models, **kwargs):
calls["n"] += 1
return ["model-a", "model-b"]
monkeypatch.setattr(
"hermes_cli.model_switch_providers._fetch_picker_live_models", fake_fetch
)
# $RANDOM would vary per call; use a counter file-free equivalent that
# is deterministic per call but different between calls.
providers = {
"gw": {
"base_url": "https://gw.example.test/v1",
"key_cmd": "printf 'tok-%s' $$", # PID: differs per invocation
"default_model": "model-a",
}
}
first = list_authenticated_providers(
user_providers=providers, refresh=True, for_picker=True
)
second = list_authenticated_providers(
user_providers=providers, refresh=True, for_picker=True
)
def row(rows):
return [r for r in rows if isinstance(r, dict) and r.get("slug") == "gw"]
assert row(first) and row(second)
assert (row(first)[0].get("models") or []) == (
row(second)[0].get("models") or []
)
class TestMintedTokenIsNeverPersisted:
"""A key_cmd token must not be written back into config.yaml.
``_model_flow_named_custom`` computes the value to persist from the
STATICALLY configured credential. Resolving key_cmd into ``api_key`` before
that call would persist a short-lived bearer, which is stale within the
hour and shadows the key_cmd that exists to re-mint it.
"""
def test_key_cmd_provider_persists_no_credential(self):
from hermes_cli.main_provider_setup import _custom_provider_api_key_config_value
assert _custom_provider_api_key_config_value(
{"key_cmd": "printf 'tok-secret'"}, ""
) == ""
def test_static_key_still_persists(self):
from hermes_cli.main_provider_setup import _custom_provider_api_key_config_value
assert _custom_provider_api_key_config_value(
{"api_key": "sk-static"}, "sk-static"
) == "sk-static"
def test_key_env_persists_as_reference(self):
"""key_env persists as ${VAR}, never the resolved secret."""
from hermes_cli.main_provider_setup import _custom_provider_api_key_config_value
assert _custom_provider_api_key_config_value(
{"key_env": "MY_KEY"}, "resolved-secret-value"
) == "${MY_KEY}"
class TestSetupFlowHonoursKeyCmd:
"""`hermes model`'s named-custom flow is the picker's sibling path.
It builds its own ``Authorization: Bearer <api_key>`` for the /models
probe, so an unresolved key_cmd sends no auth header and the endpoint
401s — same symptom, different call path.
These drive the real ``_model_flow_named_custom`` and assert on what the
probe actually receives, rather than inspecting the function's source: a
semantics-preserving refactor should not fail the suite.
"""
class _StopAfterProbe(Exception):
"""Unwind once the probe has run, before the interactive menu."""
def _run_flow_capturing_probe(self, monkeypatch, entry):
"""Invoke the flow far enough to capture the probe's credential."""
# Patch the modules the flow imports FROM, not the compat shims: the
# `hermes_cli.models` re-export of should_use_ollama_native_catalog is
# scheduled for removal (see HermesPluginCompatWarning).
import hermes_cli.models as models_mod
import hermes_cli.models_local as models_local_mod
from hermes_cli.model_setup_flows_custom import _model_flow_named_custom
seen = {}
def fake_fetch(api_key, base_url, **kwargs):
seen["api_key"] = api_key
seen["base_url"] = base_url
# The flow would prompt interactively next; stop here.
raise TestSetupFlowHonoursKeyCmd._StopAfterProbe
monkeypatch.setattr(models_mod, "fetch_api_models", fake_fetch)
# Ollama detection issues its own probe; force the generic path.
monkeypatch.setattr(
models_local_mod, "should_use_ollama_native_catalog", lambda *a, **k: False
)
try:
_model_flow_named_custom({}, dict(entry))
except TestSetupFlowHonoursKeyCmd._StopAfterProbe:
pass
except Exception:
# Any other failure still tells us what the probe received; the
# assertions below decide whether that was correct.
pass
return seen
def test_probe_receives_the_minted_token(self, monkeypatch):
seen = self._run_flow_capturing_probe(
monkeypatch,
{
"name": "gw",
"base_url": "https://gw.example.test/v1",
"key_cmd": "printf 'tok-minted'",
"model": "model-a",
},
)
assert seen.get("api_key") == "tok-minted", (
"setup flow probed with an empty key — its /models request goes "
"out unauthenticated and the endpoint 401s"
)
def test_static_api_key_still_wins(self, monkeypatch):
"""key_cmd is a fallback: an explicit api_key is used unchanged, so
existing static-key configs are unaffected."""
seen = self._run_flow_capturing_probe(
monkeypatch,
{
"name": "gw",
"base_url": "https://gw.example.test/v1",
"api_key": "sk-static",
"key_cmd": "printf 'tok-minted'",
"model": "model-a",
},
)
assert seen.get("api_key") == "sk-static"
def test_minted_token_is_not_persisted(self, monkeypatch, tmp_path):
"""The value written back to config.yaml is derived from the STATIC
credential. Persisting a short-lived bearer would leave a stale key
that also shadows the key_cmd meant to re-mint it."""
import hermes_cli.main_provider_setup as main_mod
persisted = {}
real = main_mod._custom_provider_api_key_config_value
def spy(provider_info, resolved_api_key=""):
out = real(provider_info, resolved_api_key)
persisted["value"] = out
return out
monkeypatch.setattr(main_mod, "_custom_provider_api_key_config_value", spy)
self._run_flow_capturing_probe(
monkeypatch,
{
"name": "gw",
"base_url": "https://gw.example.test/v1",
"key_cmd": "printf 'tok-minted'",
"model": "model-a",
},
)
assert persisted.get("value", "") == "", (
"a key_cmd-minted bearer reached the value persisted to config.yaml"
)
"""Command-auth discovery is lazy, credential-scoped, and never persists bearers."""
import copy
import pytest
from hermes_cli import model_switch_providers as picker
@pytest.mark.parametrize("route", ["providers", "custom_providers"])
@pytest.mark.parametrize("static", [{}, {"api_key": "stale"}, {"key_env": "TEST_GATEWAY_KEY"}])
def test_picker_command_auth_is_lazy_and_credential_scoped(monkeypatch, route, static):
from agent import command_token_source
from hermes_cli import models
monkeypatch.setenv("TEST_GATEWAY_KEY", "stale")
mints, probes = [], []
def mint(command, label):
mints.append(command)
if command == "broken":
raise RuntimeError("private helper output")
return f"token-{command}", 3600
def fetch(key, url, **kwargs):
probes.append(key)
return ["fallback", f"catalog-{key}"] if key.startswith("token-") else None
monkeypatch.setattr(command_token_source, "_mint", mint)
monkeypatch.setattr(models, "fetch_api_models", fetch)
monkeypatch.setattr("hermes_cli.models_local.should_use_ollama_native_catalog", lambda *a, **k: False)
entry = {"name": "Gateway", "base_url": "https://gateway.invalid/v1", "key_cmd": "tenant-a",
"model": "fallback", "default_model": "fallback", **static}
def rows(entries, *, probe=True):
b = picker._PickerBuild("", "", "", None, False, False, probe, False, False, set(), {})
if route == "providers":
picker._lap_user_provider_rows(b, {f"gateway-{i}": e for i, e in enumerate(entries)})
else:
picker._lap_custom_provider_rows(b, entries)
return b.results
for disabled in [False, "false"]:
assert rows([{**entry, "discover_models": disabled}])[0]["models"] == ["fallback"]
assert rows([entry], probe=False)[0]["models"] == ["fallback"]
assert not mints and not probes
first = rows([entry])[0]["models"]
assert first == ["fallback", "catalog-token-tenant-a"]
assert mints == ["tenant-a"] and probes == ["token-tenant-a"]
for probe in [False, True]:
assert rows([entry], probe=probe)[0]["models"] == first
assert mints == ["tenant-a"] and probes == ["token-tenant-a"]
from hermes_cli.config import save_config, load_config
other = {**entry, "key_cmd": "tenant-b", "discover_models": False}
save_config({"custom_providers": [entry, other]})
if route == "custom_providers":
rows([entry])
assert load_config()["custom_providers"][1].get("models") is None
source = command_token_source.build_command_token_provider("tenant-a")
empty = picker._NativePickerModelList()
result = models.cached_fetch_api_models(
source, entry["base_url"], force_refresh=True, fetch_models=lambda: empty)
assert isinstance(result, picker._NativePickerModelList) and result == []
cached_empty = models.cached_fetch_api_models(source, entry["base_url"], cache_only=True)
assert isinstance(cached_empty, picker._NativePickerModelList) and cached_empty == []
for probe in [False, True]:
native_row = rows([entry], probe=probe)[0]
assert native_row["models"] == [] and native_row["native_catalog_empty"]
models.clear_provider_models_cache()
distinct = rows([entry, {**entry, "key_cmd": "tenant-b"}])
assert [r["models"] for r in distinct] == [first, ["fallback", "catalog-token-tenant-b"]]
assert rows([{**entry, "key_cmd": "broken"}])[0]["models"] == ["fallback"]
assert "stale" not in probes
@pytest.mark.parametrize("route", ["providers", "custom_providers"])
@pytest.mark.parametrize("discover", [True, False, "false"])
@pytest.mark.parametrize("static", [{}, {"api_key": "stale"}, {"key_env": "TEST_GATEWAY_KEY"}])
def test_setup_probe_credentials_never_become_saved_credentials(monkeypatch, route, discover, static):
from agent import command_token_source
from hermes_cli import config as config_module, models
from hermes_cli import model_setup_flows_custom as setup
monkeypatch.setenv("TEST_GATEWAY_KEY", "stale")
mints, probes, choices = [], [], []
entry = {"name": "Gateway", "base_url": "https://gateway.invalid/v1", "key_cmd": "tenant-a",
"model": "fallback", "default_model": "fallback", "discover_models": discover, **static}
config = {"model": {"default": "fallback", "provider": "custom"},
route: {"gateway": copy.deepcopy(entry)} if route == "providers" else [copy.deepcopy(entry)]}
config_module.save_config(config)
from hermes_cli.main_provider_setup import _named_custom_provider_map
info = next(iter(_named_custom_provider_map(config_module.load_config()).values()))
def mint(command, label):
mints.append(command)
return "transient-bearer", 3600
def fetch(key, url, **kwargs):
probes.append(key)
return ["fallback", "discovered"] if key == "transient-bearer" else None
def pick(name, available, saved):
choices.extend(available)
return available[-1]
monkeypatch.setattr(command_token_source, "_mint", mint)
monkeypatch.setattr(models, "fetch_api_models", fetch)
monkeypatch.setattr("hermes_cli.models_local.should_use_ollama_native_catalog", lambda *a, **k: False)
monkeypatch.setattr(setup, "_pick_named_custom_model", pick)
monkeypatch.setattr(setup, "_ask", lambda *a, **k: "fallback")
setup._model_flow_named_custom(config, info)
if discover is True:
assert mints == ["tenant-a"] and probes == ["transient-bearer"]
assert choices == ["fallback", "discovered"]
else:
assert not mints and not probes
assert choices == ["fallback"]
saved = config_module.load_config()
assert "transient-bearer" not in repr(saved)
persisted = saved[route]["gateway"] if route == "providers" else saved[route][0]
assert persisted["key_cmd"] == entry["key_cmd"]
assert persisted.get("api_key", "") == static.get("api_key", "")
+9
View File
@@ -1348,6 +1348,15 @@ The command must print **only** the token on stdout: either bare, or as JSON wit
Precedence: an explicit `--api-key` flag still wins; otherwise `key_cmd` beats a static `api_key`/`key_env` on the same entry. The minted credential applies to the main agent turn and to auxiliary tasks (title generation, compression, vision, embedding) alike.
Model discovery also honors `key_cmd` for both `providers:` and legacy
`custom_providers:` entries, including `hermes model` setup. Helpers run only when
an authenticated live catalog probe is needed: disabled discovery and warm catalog
cache reads do not mint tokens. Catalogs are scoped to the command identity, so
rotating a bearer does not invalidate the catalog. Probe helpers use their own
short-lived token source, not the inference client's token cache; minted bearers
are never saved to `config.yaml`. If a helper fails, discovery falls back to the
configured model without exposing the helper's output.
Not to be confused with `secrets.command`, which runs a helper **once at startup** to populate env vars process-wide. Use that for a vault/keychain helper handing back many secrets; use `key_cmd` when one provider's credential must be re-minted *during* a session.
:::note Legacy format