From 423c14c65ae9b12108d522063333f1b567749dad Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 20:14:15 -0700 Subject: [PATCH 01/16] =?UTF-8?q?refactor(hermes=5Fcli):=20auth=5Fxai=20?= =?UTF-8?q?=E2=80=94=20shared=20JWT-exp/token-pair=20helpers,=20packed=20l?= =?UTF-8?q?ayout,=20tighter=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/auth_xai.py | 589 +++++++++++++---------------------------- 1 file changed, 191 insertions(+), 398 deletions(-) diff --git a/hermes_cli/auth_xai.py b/hermes_cli/auth_xai.py index fdd334ac1a..565aeb1e8e 100644 --- a/hermes_cli/auth_xai.py +++ b/hermes_cli/auth_xai.py @@ -14,86 +14,70 @@ import json import os import time from pathlib import Path -from typing import Any, Dict, Optional +from typing import Any, Dict, Optional, TYPE_CHECKING from urllib.parse import urlparse from hermes_cli.auth_codex import _load_auth_store_maybe_locked, _refresh_payload_access_token from hermes_cli.auth_constants import ( - AUTH_LOCK_TIMEOUT_SECONDS, - AuthError, - DEFAULT_XAI_OAUTH_BASE_URL, - DEVICE_CODE_GRANT_TYPE, - XAI_ACCESS_TOKEN_REFRESH_SKEW_SECONDS, - XAI_OAUTH_CLIENT_ID, - XAI_OAUTH_DEVICE_CODE_URL, - XAI_OAUTH_DISCOVERY_URL, - XAI_OAUTH_SCOPE, - _FORM_JSON_HEADERS, - _xai_err, - httpx, + AUTH_LOCK_TIMEOUT_SECONDS, AuthError, DEFAULT_XAI_OAUTH_BASE_URL, DEVICE_CODE_GRANT_TYPE, + XAI_ACCESS_TOKEN_REFRESH_SKEW_SECONDS, XAI_OAUTH_CLIENT_ID, XAI_OAUTH_DEVICE_CODE_URL, + XAI_OAUTH_DISCOVERY_URL, XAI_OAUTH_SCOPE, _FORM_JSON_HEADERS, _xai_err, httpx, ) from utils import env_float -from typing import TYPE_CHECKING - if TYPE_CHECKING: # annotation-only; the runtime import would be a cycle from hermes_cli.auth import ProviderConfig # Log-record parity with the origin module (caplog tests pin "hermes_cli.auth"). logger = logging.getLogger("hermes_cli.auth") +_RELOGIN = "Re-authenticate with `hermes model`." + + +def _clean(value: Any) -> str: + return str(value or "").strip() + + +def _token_pair(tokens: Any) -> tuple[str, str]: + """``(access_token, refresh_token)`` stripped; empty strings when *tokens* is not a dict.""" + if not isinstance(tokens, dict): + return "", "" + return _clean(tokens.get("access_token")), _clean(tokens.get("refresh_token")) + def _xai_oauth_state_from_store(auth_store: Dict[str, Any]) -> Optional[Dict[str, Any]]: """Return usable xAI OAuth state from provider state or credential pool.""" from hermes_cli.auth import _load_provider_state state = _load_provider_state(auth_store, "xai-oauth") - tokens = state.get("tokens") if isinstance(state, dict) else None - if isinstance(tokens, dict): - access_token = str(tokens.get("access_token", "") or "").strip() - refresh_token = str(tokens.get("refresh_token", "") or "").strip() - if access_token and refresh_token: - return state + if isinstance(state, dict) and all(_token_pair(state.get("tokens"))): + return state credential_pool = auth_store.get("credential_pool") - entries = ( - credential_pool.get("xai-oauth") - if isinstance(credential_pool, dict) - else None - ) - if isinstance(entries, list): - for entry in entries: - if not isinstance(entry, dict): - continue - access_token = str(entry.get("access_token", "") or "").strip() - refresh_token = str(entry.get("refresh_token", "") or "").strip() - if not access_token or not refresh_token: - continue - merged = dict(state or {}) - merged["tokens"] = { - "access_token": access_token, - "refresh_token": refresh_token, - "token_type": str(entry.get("token_type") or "Bearer"), - } - if entry.get("last_refresh"): - merged["last_refresh"] = entry.get("last_refresh") - merged.setdefault("auth_mode", "oauth_pkce") - return merged + entries = credential_pool.get("xai-oauth") if isinstance(credential_pool, dict) else None + for entry in entries if isinstance(entries, list) else (): + access_token, refresh_token = _token_pair(entry) + if not access_token or not refresh_token: + continue + merged = dict(state or {}) + merged["tokens"] = { + "access_token": access_token, + "refresh_token": refresh_token, + "token_type": str(entry.get("token_type") or "Bearer"), + } + if entry.get("last_refresh"): + merged["last_refresh"] = entry.get("last_refresh") + merged.setdefault("auth_mode", "oauth_pkce") + return merged return state if isinstance(state, dict) else None def _xai_oauth_state_has_usable_tokens(state: Optional[Dict[str, Any]]) -> bool: - tokens = state.get("tokens") if isinstance(state, dict) else None - return ( - isinstance(tokens, dict) - and bool(str(tokens.get("access_token", "") or "").strip()) - and bool(str(tokens.get("refresh_token", "") or "").strip()) - ) + return isinstance(state, dict) and all(_token_pair(state.get("tokens"))) def _read_xai_oauth_tokens(*, _lock: bool = True) -> Dict[str, Any]: from hermes_cli.auth import _load_global_auth_store - auth_store = _load_auth_store_maybe_locked(_lock) - state = _xai_oauth_state_from_store(auth_store) + state = _xai_oauth_state_from_store(_load_auth_store_maybe_locked(_lock)) if not _xai_oauth_state_has_usable_tokens(state): global_state = _xai_oauth_state_from_store(_load_global_auth_store()) if _xai_oauth_state_has_usable_tokens(global_state): @@ -105,22 +89,13 @@ def _read_xai_oauth_tokens(*, _lock: bool = True) -> Dict[str, Any]: ) tokens = state.get("tokens") if not isinstance(tokens, dict): - raise _xai_err( - "xAI OAuth state is missing tokens. Re-authenticate with `hermes model`.", - "xai_auth_invalid_shape", relogin=True, - ) - access_token = str(tokens.get("access_token", "") or "").strip() - refresh_token = str(tokens.get("refresh_token", "") or "").strip() - if not access_token: - raise _xai_err( - "xAI OAuth state is missing access_token. Re-authenticate with `hermes model`.", - "xai_auth_missing_access_token", relogin=True, - ) - if not refresh_token: - raise _xai_err( - "xAI OAuth state is missing refresh_token. Re-authenticate with `hermes model`.", - "xai_auth_missing_refresh_token", relogin=True, - ) + raise _xai_err(f"xAI OAuth state is missing tokens. {_RELOGIN}", "xai_auth_invalid_shape", relogin=True) + access_token, refresh_token = _token_pair(tokens) + for value, field in ((access_token, "access_token"), (refresh_token, "refresh_token")): + if not value: + raise _xai_err( + f"xAI OAuth state is missing {field}. {_RELOGIN}", f"xai_auth_missing_{field}", relogin=True, + ) return { "tokens": tokens, "last_refresh": state.get("last_refresh"), @@ -130,41 +105,30 @@ def _read_xai_oauth_tokens(*, _lock: bool = True) -> Dict[str, Any]: def _write_through_xai_oauth_to_global_root(state: Dict[str, Any]) -> None: - """Persist a rotated xAI OAuth ``state`` into the global-root auth.json. + """Persist a rotated xAI OAuth ``state`` into the global-root auth.json (best effort). - Best-effort write-through for the multi-profile rotation hazard (#43589): xAI rotates the - refresh_token on every refresh, so when a profile session refreshes a grant it resolved from the - root fallback, the rotated chain must land back in root. - - Only updates ``providers.xai-oauth`` in the root store; never touches the profile store (the - caller already saved that). Swallows all errors — a failed write-through degrades to the pre- - existing behavior (root stale), it must never break the profile's own successful save. + xAI rotates the refresh_token on every refresh, so when a profile session refreshes a grant it + resolved from the root fallback, the rotated chain must land back in root. Only updates + ``providers.xai-oauth`` in root; never touches the profile store (the caller saved that). + Swallows all errors — a failed write-through degrades to root-stale, never breaks the profile save. """ from hermes_cli.auth import _global_auth_file_path, _persist_provider_state_to_store global_path = _global_auth_file_path() - if global_path is None: - # Classic mode (profile == root); the profile save already hit root. + if global_path is None: # classic mode (profile == root); the profile save already hit root return - # Seat belt: under pytest, refuse to write the real user's - # ~/.hermes/auth.json even when HERMES_HOME points at a profile path - # (mirrors the read-side guard in _load_global_auth_store). Uses the - # unmodified HOME env, not Path.home() which fixtures may monkeypatch. - if os.environ.get("PYTEST_CURRENT_TEST"): - real_home_env = os.environ.get("HOME", "") - if real_home_env: - real_root = Path(real_home_env) / ".hermes" / "auth.json" - try: - if global_path.resolve(strict=False) == real_root.resolve(strict=False): - return - except Exception: + # Seat belt: under pytest, refuse to write the real user's ~/.hermes/auth.json even when + # HERMES_HOME points at a profile path (mirrors the read-side guard in _load_global_auth_store). + # Uses the unmodified HOME env, not Path.home() which fixtures may monkeypatch. + real_home_env = os.environ.get("HOME", "") if os.environ.get("PYTEST_CURRENT_TEST") else "" + if real_home_env: + real_root = Path(real_home_env) / ".hermes" / "auth.json" + try: + if global_path.resolve(strict=False) == real_root.resolve(strict=False): return + except Exception: + return try: - _persist_provider_state_to_store( - "xai-oauth", - state, - global_path, - set_active=False, - ) + _persist_provider_state_to_store("xai-oauth", state, global_path, set_active=False) except Exception as exc: # pragma: no cover - best effort logger.debug("xAI OAuth: write-through to global root failed: %s", exc) @@ -180,105 +144,70 @@ def _save_xai_oauth_tokens( ) -> None: """Persist xAI OAuth tokens into the auth store. - When *set_active* is True (default), also promote ``xai-oauth`` to ``active_provider`` — - appropriate for intentional model/auth login. Pass ``set_active=False`` for side-tool credential - bootstrap (TTS/setup, tools config, dashboard token save, token refresh) so inference routing is - unchanged. + *set_active* (default True) also promotes ``xai-oauth`` to ``active_provider`` — right for an + intentional login. Pass False for side-tool credential bootstrap (TTS/setup, tools config, + dashboard token save, token refresh) so inference routing is unchanged. """ from hermes_cli.auth import _auth_store_lock, _global_auth_file_path, _load_auth_store, _load_provider_state_with_source, _same_path, _save_auth_store, _store_provider_state, _utc_now_z, _write_through_xai_oauth_to_global_root if last_refresh is None: last_refresh = _utc_now_z() with _auth_store_lock(): auth_store = _load_auth_store() - # A profile that lacks its own xai-oauth block is reading the root - # grant through _load_provider_state's fallback. When such a profile - # refreshes the (rotating) grant, we must write the rotated chain back - # to root too, or root is left holding a revoked refresh token (#43589). - # #74339: the old key-presence check (_profile_has_own_xai_oauth_state) - # decided write-through based on whether the profile had a - # providers.xai-oauth key BEFORE the save — but _store_provider_state - # unconditionally creates that key below. Use - # _load_provider_state_with_source to learn where the grant was - # resolved from and write back only to that source. - state, source_path = _load_provider_state_with_source( - auth_store, "xai-oauth" - ) - if state is None: - state = {} - state["tokens"] = tokens - state["last_refresh"] = last_refresh - state["auth_mode"] = auth_mode + # A profile without its own xai-oauth block reads the root grant through + # _load_provider_state's fallback. Refreshing that (rotating) grant must write the rotated + # chain back to root, or root keeps a revoked refresh token. Decide by where the grant was + # resolved FROM, not by key presence: _store_provider_state below would create the key. + state, source_path = _load_provider_state_with_source(auth_store, "xai-oauth") + state = state if state is not None else {} + state.update(tokens=tokens, last_refresh=last_refresh, auth_mode=auth_mode) if discovery: state["discovery"] = discovery if redirect_uri: state["redirect_uri"] = redirect_uri global_root = _global_auth_file_path() - is_from_root = bool( - source_path is not None - and global_root is not None - and _same_path(source_path, global_root) - ) - if is_from_root: - # Grant was resolved from root — write back to root only. - # Do NOT call _store_provider_state on the profile auth_store - # (it would create a shadowing providers.xai-oauth key that - # disables write-through on the next refresh — #74339). + if source_path is not None and global_root is not None and _same_path(source_path, global_root): + # Resolved from root — write back to root only. Storing on the profile auth_store would + # create a shadowing providers.xai-oauth key that disables write-through next refresh. _write_through_xai_oauth_to_global_root(state) else: - # Profile genuinely owns this — write to profile store. - _store_provider_state( - auth_store, "xai-oauth", state, set_active=set_active - ) + _store_provider_state(auth_store, "xai-oauth", state, set_active=set_active) _save_auth_store(auth_store) -def _xai_access_token_is_expiring(access_token: str, skew_seconds: int = 0) -> bool: +def _xai_jwt_exp(access_token: Any) -> Optional[float]: + """``exp`` claim of a JWT-shaped access token, or None when absent/undecodable.""" if not isinstance(access_token, str) or "." not in access_token: - return False + return None try: parts = access_token.split(".") if len(parts) < 2: - return False - payload_b64 = parts[1] - payload_b64 += "=" * (-len(payload_b64) % 4) - payload = json.loads(base64.urlsafe_b64decode(payload_b64.encode("ascii")).decode("utf-8")) - exp = payload.get("exp") - if not isinstance(exp, (int, float)): - return False - return float(exp) <= (time.time() + max(0, int(skew_seconds))) + return None + payload_b64 = parts[1] + "=" * (-len(parts[1]) % 4) + exp = json.loads(base64.urlsafe_b64decode(payload_b64.encode("ascii")).decode("utf-8")).get("exp") + return float(exp) if isinstance(exp, (int, float)) else None except Exception: - return False + return None + + +def _xai_access_token_is_expiring(access_token: str, skew_seconds: int = 0) -> bool: + exp = _xai_jwt_exp(access_token) + return exp is not None and exp <= time.time() + max(0, int(skew_seconds)) def _xai_proactive_refresh_skew_seconds(access_token: str) -> int: """How far before JWT ``exp`` to proactively refresh xAI OAuth tokens. - SuperGrok sessions ship multi-hour tokens where the gateway-oriented hour-long skew makes sense, - but device-code logins often return ~15-minute JWTs; the full skew would force a refresh on - every credential resolution, burning single-use refresh tokens and racing concurrent callers - into ``invalid_grant`` quarantine. + SuperGrok sessions ship multi-hour tokens where the hour-long skew makes sense, but device-code + logins often return ~15-minute JWTs; the full skew would force a refresh on every credential + resolution, burning single-use refresh tokens and racing concurrent callers into + ``invalid_grant`` quarantine. """ max_skew = XAI_ACCESS_TOKEN_REFRESH_SKEW_SECONDS - if not isinstance(access_token, str) or "." not in access_token: - return max_skew - try: - parts = access_token.split(".") - if len(parts) < 2: - return max_skew - payload_b64 = parts[1] - payload_b64 += "=" * (-len(payload_b64) % 4) - payload = json.loads(base64.urlsafe_b64decode(payload_b64.encode("ascii")).decode("utf-8")) - exp = payload.get("exp") - if not isinstance(exp, (int, float)): - return max_skew - remaining = float(exp) - time.time() - if remaining <= 0: - return max_skew - if remaining <= 45 * 60: - return min(120, max_skew) - return max_skew - except Exception: + exp = _xai_jwt_exp(access_token) + if exp is None: return max_skew + remaining = exp - time.time() + return min(120, max_skew) if 0 < remaining <= 45 * 60 else max_skew def _is_xai_origin_host(host: str) -> bool: @@ -290,22 +219,15 @@ def _xai_validate_oauth_endpoint(url: str, *, field: str) -> str: """Refuse any OIDC discovery endpoint that isn't HTTPS on the xAI origin. The discovery result is cached in auth.json, so a single MITM at login could plant a malicious - ``token_endpoint`` that receives the refresh_token forever. Pinning scheme + host (RFC 8414 §2: - HTTPS issuer, same-origin token_endpoint) removes that persistence; ``x.ai`` is the bare apex, - so an exact match or any ``.x.ai`` suffix is accepted. + ``token_endpoint`` that receives the refresh_token forever. Pinning scheme + host (RFC 8414 §2) + removes that persistence. """ parsed = urlparse(url) if parsed.scheme != "https": - raise _xai_err( - f"xAI OIDC discovery returned a non-HTTPS {field}: {url!r}.", - "xai_discovery_invalid", - ) + raise _xai_err(f"xAI OIDC discovery returned a non-HTTPS {field}: {url!r}.", "xai_discovery_invalid") host = (parsed.hostname or "").lower() if not host: - raise _xai_err( - f"xAI OIDC discovery {field} is missing a hostname: {url!r}.", - "xai_discovery_invalid", - ) + raise _xai_err(f"xAI OIDC discovery {field} is missing a hostname: {url!r}.", "xai_discovery_invalid") if not _is_xai_origin_host(host): raise _xai_err( f"xAI OIDC discovery {field} host {host!r} is not on the xAI origin " @@ -320,12 +242,9 @@ def _xai_validate_oauth_endpoint(url: str, *, field: str) -> str: def _xai_validate_inference_base_url(value: str, *, fallback: str) -> str: """Refuse a non-xAI base_url for the OAuth-authenticated inference path. - Pin the inference origin to ``api.x.ai`` (or any ``*.x.ai`` subdomain xAI may add). On - rejection, fall back to the default and log a warning rather than raise — a bad env var should - not deadlock authentication, but it should also never leak the bearer. - - ``value`` is the already-stripped, trailing-slash-trimmed candidate from env. Empty input - returns ``fallback`` unchanged. + Pins the inference origin to ``api.x.ai`` (or any ``*.x.ai`` subdomain). On rejection, fall + back to the default and log a warning rather than raise — a bad env var should not deadlock + authentication, but it must never leak the bearer. Empty input returns ``fallback``. """ candidate = (value or "").strip().rstrip("/") if not candidate: @@ -333,10 +252,7 @@ def _xai_validate_inference_base_url(value: str, *, fallback: str) -> str: try: parsed = urlparse(candidate) except Exception: - logger.warning( - "Ignoring malformed xAI base_url override %r; using %s instead.", - candidate, fallback, - ) + logger.warning("Ignoring malformed xAI base_url override %r; using %s instead.", candidate, fallback) return fallback if parsed.scheme != "https": logger.warning( @@ -347,10 +263,7 @@ def _xai_validate_inference_base_url(value: str, *, fallback: str) -> str: return fallback host = (parsed.hostname or "").lower() if not host: - logger.warning( - "Ignoring xAI base_url override %r with no hostname; using %s instead.", - candidate, fallback, - ) + logger.warning("Ignoring xAI base_url override %r with no hostname; using %s instead.", candidate, fallback) return fallback if not _is_xai_origin_host(host): logger.warning( @@ -366,43 +279,23 @@ def _xai_validate_inference_base_url(value: str, *, fallback: str) -> str: def _xai_oauth_discovery(timeout_seconds: float = 15.0) -> Dict[str, str]: try: - response = httpx.get( - XAI_OAUTH_DISCOVERY_URL, - headers={"Accept": "application/json"}, - timeout=timeout_seconds, - ) + response = httpx.get(XAI_OAUTH_DISCOVERY_URL, headers={"Accept": "application/json"}, timeout=timeout_seconds) except Exception as exc: raise _xai_err(f"xAI OIDC discovery failed: {exc}", "xai_discovery_failed") from exc if response.status_code != 200: - raise _xai_err( - f"xAI OIDC discovery returned status {response.status_code}.", - "xai_discovery_failed", - ) + raise _xai_err(f"xAI OIDC discovery returned status {response.status_code}.", "xai_discovery_failed") try: payload = response.json() except Exception as exc: - raise _xai_err( - f"xAI OIDC discovery returned invalid JSON: {exc}", - "xai_discovery_invalid_json", - ) from exc + raise _xai_err(f"xAI OIDC discovery returned invalid JSON: {exc}", "xai_discovery_invalid_json") from exc if not isinstance(payload, dict): - raise _xai_err( - "xAI OIDC discovery response was not a JSON object.", - "xai_discovery_incomplete", - ) - authorization_endpoint = str(payload.get("authorization_endpoint", "") or "").strip() - token_endpoint = str(payload.get("token_endpoint", "") or "").strip() - if not authorization_endpoint or not token_endpoint: - raise _xai_err( - "xAI OIDC discovery response was missing required endpoints.", - "xai_discovery_incomplete", - ) - _xai_validate_oauth_endpoint(authorization_endpoint, field="authorization_endpoint") - _xai_validate_oauth_endpoint(token_endpoint, field="token_endpoint") - return { - "authorization_endpoint": authorization_endpoint, - "token_endpoint": token_endpoint, - } + raise _xai_err("xAI OIDC discovery response was not a JSON object.", "xai_discovery_incomplete") + endpoints = {k: _clean(payload.get(k)) for k in ("authorization_endpoint", "token_endpoint")} + if not all(endpoints.values()): + raise _xai_err("xAI OIDC discovery response was missing required endpoints.", "xai_discovery_incomplete") + for field, url in endpoints.items(): + _xai_validate_oauth_endpoint(url, field=field) + return endpoints def _xai_tokens_from_payload(payload: Dict[str, Any], access_token: str, fallback_refresh: str) -> Dict[str, Any]: @@ -410,9 +303,9 @@ def _xai_tokens_from_payload(payload: Dict[str, Any], access_token: str, fallbac return { "access_token": access_token, "refresh_token": str(payload.get("refresh_token") or fallback_refresh).strip(), - "id_token": str(payload.get("id_token") or "").strip(), + "id_token": _clean(payload.get("id_token")), "expires_in": payload.get("expires_in"), - "token_type": str(payload.get("token_type") or "Bearer").strip() or "Bearer", + "token_type": _clean(payload.get("token_type") or "Bearer") or "Bearer", } @@ -427,40 +320,29 @@ def refresh_xai_oauth_pure( del access_token if not _nonempty_str(refresh_token): raise _xai_err( - "xAI OAuth is missing refresh_token. Re-authenticate with `hermes model`.", - "xai_auth_missing_refresh_token", relogin=True, + f"xAI OAuth is missing refresh_token. {_RELOGIN}", "xai_auth_missing_refresh_token", relogin=True, ) endpoint = token_endpoint.strip() or _xai_oauth_discovery(timeout_seconds)["token_endpoint"] - # Re-validate cached endpoints on the refresh hot path: an auth.json - # written by an older Hermes (or hand-edited) may carry a non-xAI - # token_endpoint that would receive every future refresh_token in - # plaintext if we trusted it blindly. Cheap suffix check; fast-fail - # with a clear error so the user can re-run `hermes model` to refetch. + # Re-validate cached endpoints on the refresh hot path: an auth.json written by an older Hermes + # (or hand-edited) may carry a non-xAI token_endpoint that would receive every future + # refresh_token in plaintext if trusted blindly. _xai_validate_oauth_endpoint(endpoint, field="token_endpoint") timeout = httpx.Timeout(max(5.0, float(timeout_seconds))) with httpx.Client(timeout=timeout, headers={"Accept": "application/json"}) as client: response = client.post( endpoint, headers={"Content-Type": "application/x-www-form-urlencoded"}, - data={ - "grant_type": "refresh_token", - "client_id": XAI_OAUTH_CLIENT_ID, - "refresh_token": refresh_token, - }, + data={"grant_type": "refresh_token", "client_id": XAI_OAUTH_CLIENT_ID, "refresh_token": refresh_token}, ) if response.status_code != 200: detail = response.text.strip() - # ``403`` from xAI's token endpoint is almost always a tier / - # entitlement gate (the OAuth grant exists but the account isn't - # on the allowlist for API access). Re-running ``hermes model`` - # won't fix that — surface a separate error code so - # ``format_auth_error`` doesn't append a misleading - # re-authenticate hint, and point users at the ``XAI_API_KEY`` - # fallback. See #26847. + suffix = f" Response: {detail}" if detail else "" + # 403 from xAI's token endpoint is almost always a tier / entitlement gate (the grant exists + # but the account isn't allowlisted for API access). Re-running `hermes model` won't fix + # that — use a separate code so format_auth_error skips the re-authenticate hint. if response.status_code == 403: raise _xai_err( - "xAI token refresh failed with HTTP 403." - + (f" Response: {detail}" if detail else "") + "xAI token refresh failed with HTTP 403." + suffix + " This OAuth account is not authorized for xAI API" " access — xAI may be restricting API/OAuth use to" " specific SuperGrok tiers despite the in-app" @@ -471,9 +353,7 @@ def refresh_xai_oauth_pure( "xai_oauth_tier_denied", relogin=False, ) raise _xai_err( - "xAI token refresh failed." - + (f" Response: {detail}" if detail else ""), - "xai_refresh_failed", relogin=response.status_code in {400, 401}, + "xAI token refresh failed." + suffix, "xai_refresh_failed", relogin=response.status_code in {400, 401}, ) payload, refreshed_access = _refresh_payload_access_token( response, @@ -481,19 +361,10 @@ def refresh_xai_oauth_pure( invalid_json=("xAI token refresh returned invalid JSON: {exc}", "xai_refresh_invalid_json"), invalid_json_relogin=False, strict_str=False, - invalid_response=( - "xAI token refresh response was not a JSON object.", - "xai_refresh_invalid_response", - ), - missing_access=( - "xAI token refresh response was missing access_token.", - "xai_refresh_missing_access_token", - ), + invalid_response=("xAI token refresh response was not a JSON object.", "xai_refresh_invalid_response"), + missing_access=("xAI token refresh response was missing access_token.", "xai_refresh_missing_access_token"), ) - return { - **_xai_tokens_from_payload(payload, refreshed_access, refresh_token), - "last_refresh": _utc_now_z(), - } + return {**_xai_tokens_from_payload(payload, refreshed_access, refresh_token), "last_refresh": _utc_now_z()} def _refresh_xai_oauth_tokens( @@ -503,9 +374,8 @@ def _refresh_xai_oauth_tokens( redirect_uri: str = "", timeout_seconds: float, ) -> Dict[str, Any]: - # Re-persist whatever auth_mode is already stored (legacy pre-device-code - # logins may still carry ``oauth_pkce``): the refresh hot path must not - # relabel how the grant was originally obtained. + # Re-persist whatever auth_mode is already stored (legacy pre-device-code logins may still + # carry ``oauth_pkce``): the refresh hot path must not relabel how the grant was obtained. from hermes_cli.auth import _load_auth_store, _load_provider_state, refresh_xai_oauth_pure try: state = _load_provider_state(_load_auth_store(), "xai-oauth") or {} @@ -513,10 +383,8 @@ def _refresh_xai_oauth_tokens( except Exception: auth_mode = "oauth_device_code" refreshed = refresh_xai_oauth_pure( - str(tokens.get("access_token", "") or ""), - str(tokens.get("refresh_token", "") or ""), - token_endpoint=token_endpoint, - timeout_seconds=timeout_seconds, + _clean(tokens.get("access_token")), _clean(tokens.get("refresh_token")), + token_endpoint=token_endpoint, timeout_seconds=timeout_seconds, ) updated_tokens = dict(tokens) updated_tokens["access_token"] = refreshed["access_token"] @@ -527,15 +395,11 @@ def _refresh_xai_oauth_tokens( updated_tokens["expires_in"] = refreshed["expires_in"] if refreshed.get("token_type"): updated_tokens["token_type"] = refreshed["token_type"] + # set_active=False: refresh must not flip active_provider — TTS/side tools can refresh xAI + # tokens while chat still routes through another provider. _save_xai_oauth_tokens( - updated_tokens, - discovery={"token_endpoint": token_endpoint}, - redirect_uri=redirect_uri, - last_refresh=refreshed["last_refresh"], - auth_mode=auth_mode, - # Refresh must not flip active_provider — TTS/side tools can refresh - # xAI tokens while chat still routes through another provider. - set_active=False, + updated_tokens, discovery={"token_endpoint": token_endpoint}, redirect_uri=redirect_uri, + last_refresh=refreshed["last_refresh"], auth_mode=auth_mode, set_active=False, ) return updated_tokens @@ -544,33 +408,29 @@ def _quarantine_xai_oauth_tokens(exc: AuthError) -> None: """Clear dead xAI tokens from auth.json after a terminal refresh failure. Terminal = HTTP 400/401/403 (invalid_grant, token revoked). Subsequent sessions then fail fast - without a network retry. Mirrors credential_pool.py quarantine. Best-effort: persistence - failures are logged and swallowed (caller re-raises the original error regardless). + without a network retry. Best-effort: persistence failures are logged and swallowed (caller + re-raises the original error regardless). """ from hermes_cli.auth import _last_auth_error_marker, _load_auth_store, _load_provider_state, _save_auth_store, _store_provider_state try: - _q_store = _load_auth_store() - _q_state = _load_provider_state(_q_store, "xai-oauth") or {} - _q_tokens = dict(_q_state.get("tokens") or {}) - _q_tokens.pop("access_token", None) - _q_tokens.pop("refresh_token", None) - _q_state["tokens"] = _q_tokens - _q_state["last_auth_error"] = _last_auth_error_marker( - "xai-oauth", exc, - reason="runtime_refresh_failure", default_code="xai_refresh_failed", - ) - _store_provider_state(_q_store, "xai-oauth", _q_state, set_active=False) - _save_auth_store(_q_store) - except Exception as _save_exc: - logger.debug( - "xAI OAuth: failed to persist quarantined state: %s", _save_exc, + store = _load_auth_store() + state = _load_provider_state(store, "xai-oauth") or {} + tokens = dict(state.get("tokens") or {}) + tokens.pop("access_token", None) + tokens.pop("refresh_token", None) + state["tokens"] = tokens + state["last_auth_error"] = _last_auth_error_marker( + "xai-oauth", exc, reason="runtime_refresh_failure", default_code="xai_refresh_failed", ) + _store_provider_state(store, "xai-oauth", state, set_active=False) + _save_auth_store(store) + except Exception as save_exc: + logger.debug("xAI OAuth: failed to persist quarantined state: %s", save_exc) def _xai_oauth_inference_base_url() -> str: return _xai_validate_inference_base_url( - os.getenv("HERMES_XAI_BASE_URL", "").strip().rstrip("/") - or os.getenv("XAI_BASE_URL", "").strip().rstrip("/"), + os.getenv("HERMES_XAI_BASE_URL", "").strip().rstrip("/") or os.getenv("XAI_BASE_URL", "").strip().rstrip("/"), fallback=DEFAULT_XAI_OAUTH_BASE_URL, ) @@ -582,20 +442,19 @@ def resolve_xai_oauth_runtime_credentials( refresh_skew_seconds: Optional[int] = None, ) -> Dict[str, Any]: from hermes_cli.auth import _auth_store_lock, _is_terminal_xai_oauth_refresh_error, _refresh_xai_oauth_tokens, _xai_oauth_discovery + def _view(data: Dict[str, Any]) -> tuple[Dict[str, Any], str, str, str, bool]: tokens = dict(data["tokens"]) - access_token = str(tokens.get("access_token", "") or "").strip() - discovery = dict(data.get("discovery") or {}) - token_endpoint = str(discovery.get("token_endpoint", "") or "").strip() - redirect_uri = str(data.get("redirect_uri", "") or "").strip() + access_token = _clean(tokens.get("access_token")) + token_endpoint = _clean(dict(data.get("discovery") or {}).get("token_endpoint")) + redirect_uri = _clean(data.get("redirect_uri")) effective_skew = ( - int(refresh_skew_seconds) - if refresh_skew_seconds is not None + int(refresh_skew_seconds) if refresh_skew_seconds is not None else _xai_proactive_refresh_skew_seconds(access_token) ) - should_refresh = bool(force_refresh) - if (not should_refresh) and refresh_if_expiring: - should_refresh = _xai_access_token_is_expiring(access_token, effective_skew) + should_refresh = bool(force_refresh) or bool( + refresh_if_expiring and _xai_access_token_is_expiring(access_token, effective_skew) + ) return tokens, access_token, token_endpoint, redirect_uri, should_refresh data = _read_xai_oauth_tokens() @@ -610,37 +469,28 @@ def resolve_xai_oauth_runtime_credentials( token_endpoint = _xai_oauth_discovery(refresh_timeout_seconds)["token_endpoint"] try: tokens = _refresh_xai_oauth_tokens( - tokens, - token_endpoint=token_endpoint, - redirect_uri=redirect_uri, + tokens, token_endpoint=token_endpoint, redirect_uri=redirect_uri, timeout_seconds=refresh_timeout_seconds, ) - access_token = str(tokens.get("access_token", "") or "").strip() + access_token = _clean(tokens.get("access_token")) except AuthError as exc: if _is_terminal_xai_oauth_refresh_error(exc): _quarantine_xai_oauth_tokens(exc) raise - base_url = _xai_oauth_inference_base_url() return { "provider": "xai-oauth", - "base_url": base_url, + "base_url": _xai_oauth_inference_base_url(), "api_key": access_token, "source": "hermes-auth-store", "last_refresh": data.get("last_refresh"), - # Display/telemetry only. Device-code is the only supported xAI OAuth - # flow, so report it unconditionally — auth.json may still carry a - # legacy ``oauth_pkce`` label, which the refresh path preserves as-is. + # Display/telemetry only. Device-code is the only supported xAI OAuth flow, so report it + # unconditionally — auth.json may still carry a legacy ``oauth_pkce`` label. "auth_mode": "oauth_device_code", } -def _login_xai_oauth( - args, - pconfig: ProviderConfig, - *, - force_new_login: bool = False, -) -> None: +def _login_xai_oauth(args, pconfig: ProviderConfig, *, force_new_login: bool = False) -> None: from hermes_cli.auth import _is_remote_session, _offer_existing_oauth_credentials, _print_login_success, _update_config_for_provider, _xai_oauth_device_code_login, resolve_xai_oauth_runtime_credentials, unsuppress_credential_source del pconfig @@ -663,10 +513,7 @@ def _login_xai_oauth( if _is_remote_session(): open_browser = False - creds = _xai_oauth_device_code_login( - timeout_seconds=timeout_seconds, - open_browser=open_browser, - ) + creds = _xai_oauth_device_code_login(timeout_seconds=timeout_seconds, open_browser=open_browser) _save_xai_oauth_tokens( creds["tokens"], discovery=creds.get("discovery"), @@ -674,31 +521,19 @@ def _login_xai_oauth( last_refresh=creds.get("last_refresh"), auth_mode="oauth_device_code", ) - # An explicit interactive re-login is a strong signal the user wants the - # xAI credential re-enabled. ``hermes auth remove xai-oauth`` leaves a - # ``device_code`` suppression marker that otherwise stops the singleton - # seed from re-creating the pool entry, so ``hermes auth list`` would show - # nothing even though the agent still works via the singleton fallback. - # Clear it here (same helper ``auth_add_command`` uses). This is kept OUT - # of ``_save_xai_oauth_tokens`` on purpose — that helper is shared with the - # refresh hot path, which must never mutate suppression state. + # An explicit interactive re-login means the user wants the xAI credential re-enabled. + # ``hermes auth remove xai-oauth`` leaves a ``device_code`` suppression marker that otherwise + # stops the singleton seed from re-creating the pool entry. Kept OUT of _save_xai_oauth_tokens + # on purpose — that helper is shared with the refresh hot path, which must never mutate + # suppression state. unsuppress_credential_source("xai-oauth", "device_code") config_path = _update_config_for_provider("xai-oauth", creds.get("base_url", DEFAULT_XAI_OAUTH_BASE_URL)) _print_login_success("xai-oauth", config_path, show_auth_state=True) -def _xai_oauth_request_device_code( - client: httpx.Client, - *, - scope: str = XAI_OAUTH_SCOPE, -) -> Dict[str, Any]: +def _xai_oauth_request_device_code(client: httpx.Client, *, scope: str = XAI_OAUTH_SCOPE) -> Dict[str, Any]: response = client.post( - XAI_OAUTH_DEVICE_CODE_URL, - headers=_FORM_JSON_HEADERS, - data={ - "client_id": XAI_OAUTH_CLIENT_ID, - "scope": scope, - }, + XAI_OAUTH_DEVICE_CODE_URL, headers=_FORM_JSON_HEADERS, data={"client_id": XAI_OAUTH_CLIENT_ID, "scope": scope}, ) if response.status_code != 200: raise _xai_err( @@ -707,20 +542,10 @@ def _xai_oauth_request_device_code( "device_code_request_failed", ) payload = response.json() - required = ( - "device_code", - "user_code", - "verification_uri", - "verification_uri_complete", - "expires_in", - "interval", - ) + required = ("device_code", "user_code", "verification_uri", "verification_uri_complete", "expires_in", "interval") missing = [key for key in required if key not in payload] if missing: - raise _xai_err( - f"xAI device-code response missing fields: {', '.join(missing)}", - "device_code_invalid", - ) + raise _xai_err(f"xAI device-code response missing fields: {', '.join(missing)}", "device_code_invalid") return payload @@ -733,6 +558,7 @@ def _xai_oauth_poll_device_token( poll_interval: int, ) -> Dict[str, Any]: from hermes_cli.auth import _poll_device_token_generic + def _validate(payload: Dict[str, Any]) -> None: for field_name, article in (("access_token", "an"), ("refresh_token", "a")): if not payload.get(field_name): @@ -742,89 +568,56 @@ def _xai_oauth_poll_device_token( ) def _error(response, error_payload) -> Exception: - description = ( - error_payload.get("error_description") - or error_payload.get("error") - or response.text - ) - return _xai_err( - f"xAI device-code token polling failed: {description}", - "xai_device_token_failed", - ) + description = error_payload.get("error_description") or error_payload.get("error") or response.text + return _xai_err(f"xAI device-code token polling failed: {description}", "xai_device_token_failed") return _poll_device_token_generic( lambda: client.post( token_endpoint, headers=_FORM_JSON_HEADERS, - data={ - "grant_type": DEVICE_CODE_GRANT_TYPE, - "client_id": XAI_OAUTH_CLIENT_ID, - "device_code": device_code, - }, + data={"grant_type": DEVICE_CODE_GRANT_TYPE, "client_id": XAI_OAUTH_CLIENT_ID, "device_code": device_code}, ), expires_in=int(expires_in), poll_interval=max(1, int(poll_interval)), validate_success=_validate, on_non_json_error=lambda _r: _xai_err( - "xAI device-code token polling returned a non-JSON error response.", - "xai_device_token_failed", + "xAI device-code token polling returned a non-JSON error response.", "xai_device_token_failed", ), on_error=_error, - on_timeout=lambda: _xai_err( - "Timed out waiting for xAI device authorization.", - "device_code_timeout", - ), + on_timeout=lambda: _xai_err("Timed out waiting for xAI device authorization.", "device_code_timeout"), ) -def _xai_oauth_device_code_login( - *, - timeout_seconds: float = 20.0, - open_browser: bool = True, -) -> Dict[str, Any]: +def _xai_oauth_device_code_login(*, timeout_seconds: float = 20.0, open_browser: bool = True) -> Dict[str, Any]: from hermes_cli.auth import _can_open_graphical_browser, _is_remote_session, _print_device_code_instructions, _utc_now_z, _xai_oauth_discovery, _xai_oauth_poll_device_token discovery = _xai_oauth_discovery(timeout_seconds) - token_endpoint = discovery["token_endpoint"] timeout = httpx.Timeout(max(20.0, timeout_seconds)) with httpx.Client(timeout=timeout, headers={"Accept": "application/json"}) as client: device_data = _xai_oauth_request_device_code(client) - verification_url = str( - device_data.get("verification_uri_complete") - or device_data["verification_uri"] - ) - user_code = str(device_data["user_code"]) - expires_in = int(device_data["expires_in"]) interval = int(device_data["interval"]) - _print_device_code_instructions( - verification_url, - user_code, + str(device_data.get("verification_uri_complete") or device_data["verification_uri"]), + str(device_data["user_code"]), open_browser=open_browser and not _is_remote_session() and _can_open_graphical_browser(), swallow_open_errors=True, ) print(f"Waiting for approval (polling every {max(1, interval)}s)...") - payload = _xai_oauth_poll_device_token( client, - token_endpoint=token_endpoint, + token_endpoint=discovery["token_endpoint"], device_code=str(device_data["device_code"]), - expires_in=expires_in, + expires_in=int(device_data["expires_in"]), poll_interval=interval, ) - access_token = str(payload.get("access_token", "") or "").strip() - refresh_token = str(payload.get("refresh_token", "") or "").strip() + access_token, refresh_token = _token_pair(payload) if not access_token or not refresh_token: - raise _xai_err( - "xAI device-code token response was missing required tokens.", - "xai_device_token_invalid", - ) - base_url = _xai_oauth_inference_base_url() + raise _xai_err("xAI device-code token response was missing required tokens.", "xai_device_token_invalid") return { "tokens": _xai_tokens_from_payload(payload, access_token, refresh_token), "discovery": discovery, "redirect_uri": "", - "base_url": base_url, + "base_url": _xai_oauth_inference_base_url(), "last_refresh": _utc_now_z(), "source": "oauth-device-code", } From 997adbcafc6cb8b59afc9336b84bf57fa2c45328 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 20:19:36 -0700 Subject: [PATCH 02/16] =?UTF-8?q?refactor(hermes=5Fcli):=20auth=5Fspotify?= =?UTF-8?q?=20=E2=80=94=20fold=20print=20ladders,=20dedupe=20scope/redirec?= =?UTF-8?q?t=20validation,=20packed=20layout?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/auth_spotify.py | 252 +++++++++++++------------------------ 1 file changed, 86 insertions(+), 166 deletions(-) diff --git a/hermes_cli/auth_spotify.py b/hermes_cli/auth_spotify.py index 1de2a39d68..f8fd068e93 100644 --- a/hermes_cli/auth_spotify.py +++ b/hermes_cli/auth_spotify.py @@ -21,32 +21,23 @@ from http.server import BaseHTTPRequestHandler, HTTPServer from typing import Any, Dict, List, Optional, Tuple from urllib.parse import parse_qs, urlencode, urlparse from hermes_cli.auth_constants import ( - AuthError, - DEFAULT_SPOTIFY_ACCOUNTS_BASE_URL, - DEFAULT_SPOTIFY_API_BASE_URL, - DEFAULT_SPOTIFY_REDIRECT_URI, - DEFAULT_SPOTIFY_SCOPE, - SPOTIFY_ACCESS_TOKEN_REFRESH_SKEW_SECONDS, - SPOTIFY_DASHBOARD_URL, - SPOTIFY_DOCS_URL, - _spotify_err, - httpx, + AuthError, DEFAULT_SPOTIFY_ACCOUNTS_BASE_URL, DEFAULT_SPOTIFY_API_BASE_URL, DEFAULT_SPOTIFY_REDIRECT_URI, + DEFAULT_SPOTIFY_SCOPE, SPOTIFY_ACCESS_TOKEN_REFRESH_SKEW_SECONDS, SPOTIFY_DASHBOARD_URL, SPOTIFY_DOCS_URL, + _spotify_err, httpx, ) # Log-record parity with the origin module (caplog tests pin "hermes_cli.auth"). logger = logging.getLogger("hermes_cli.auth") +_CALLBACK_HTML = "

Spotify authorization {}.

You can close this tab." + + +def _clean(value: Any) -> str: + return str(value or "").strip() + def _spotify_scope_list(raw_scope: Optional[str] = None) -> List[str]: - scope_text = (raw_scope or DEFAULT_SPOTIFY_SCOPE).strip() - scopes = [part for part in scope_text.split() if part] - seen: set[str] = set() - ordered: List[str] = [] - for scope in scopes: - if scope not in seen: - seen.add(scope) - ordered.append(scope) - return ordered + return list(dict.fromkeys((raw_scope or DEFAULT_SPOTIFY_SCOPE).split())) def _spotify_scope_string(raw_scope: Optional[str] = None) -> str: @@ -72,7 +63,7 @@ def _spotify_setting( default, ) for candidate in candidates: - cleaned = str(candidate or "").strip() + cleaned = _clean(candidate) if strip_slash: cleaned = cleaned.rstrip("/") if cleaned: @@ -80,10 +71,7 @@ def _spotify_setting( return default -def _spotify_client_id( - explicit: Optional[str] = None, - state: Optional[Dict[str, Any]] = None, -) -> str: +def _spotify_client_id(explicit: Optional[str] = None, state: Optional[Dict[str, Any]] = None) -> str: client_id = _spotify_setting( state, "client_id", ("HERMES_SPOTIFY_CLIENT_ID", "SPOTIFY_CLIENT_ID"), "", explicit=explicit, ) @@ -95,10 +83,7 @@ def _spotify_client_id( ) -def _spotify_redirect_uri( - explicit: Optional[str] = None, - state: Optional[Dict[str, Any]] = None, -) -> str: +def _spotify_redirect_uri(explicit: Optional[str] = None, state: Optional[Dict[str, Any]] = None) -> str: return _spotify_setting( state, "redirect_uri", ("HERMES_SPOTIFY_REDIRECT_URI", "SPOTIFY_REDIRECT_URI"), DEFAULT_SPOTIFY_REDIRECT_URI, explicit=explicit, @@ -107,21 +92,19 @@ def _spotify_redirect_uri( def _spotify_api_base_url(state: Optional[Dict[str, Any]] = None) -> str: return _spotify_setting( - state, "api_base_url", ("HERMES_SPOTIFY_API_BASE_URL",), - DEFAULT_SPOTIFY_API_BASE_URL, strip_slash=True, + state, "api_base_url", ("HERMES_SPOTIFY_API_BASE_URL",), DEFAULT_SPOTIFY_API_BASE_URL, strip_slash=True, ) def _spotify_accounts_base_url(state: Optional[Dict[str, Any]] = None) -> str: return _spotify_setting( - state, "accounts_base_url", ("HERMES_SPOTIFY_ACCOUNTS_BASE_URL",), - DEFAULT_SPOTIFY_ACCOUNTS_BASE_URL, strip_slash=True, + state, "accounts_base_url", ("HERMES_SPOTIFY_ACCOUNTS_BASE_URL",), DEFAULT_SPOTIFY_ACCOUNTS_BASE_URL, + strip_slash=True, ) def _spotify_code_verifier(length: int = 64) -> str: - raw = base64.urlsafe_b64encode(os.urandom(length)).decode("ascii") - return raw.rstrip("=")[:128] + return base64.urlsafe_b64encode(os.urandom(length)).decode("ascii").rstrip("=")[:128] def _spotify_code_challenge(code_verifier: str) -> str: @@ -152,32 +135,20 @@ def _spotify_build_authorize_url( def _spotify_validate_redirect_uri(redirect_uri: str) -> tuple[str, int, str]: parsed = urlparse(redirect_uri) - if parsed.scheme != "http": - raise _spotify_err( - "Spotify PKCE redirect_uri must use http://localhost or http://127.0.0.1.", - "spotify_redirect_invalid", - ) host = parsed.hostname or "" - if host not in {"127.0.0.1", "localhost"}: - raise _spotify_err( - "Spotify PKCE redirect_uri must point to localhost or 127.0.0.1.", - "spotify_redirect_invalid", - ) - if not parsed.port: - raise _spotify_err( - "Spotify PKCE redirect_uri must include an explicit localhost port.", - "spotify_redirect_invalid", - ) + problem = ( + "must use http://localhost or http://127.0.0.1." if parsed.scheme != "http" + else "must point to localhost or 127.0.0.1." if host not in {"127.0.0.1", "localhost"} + else "must include an explicit localhost port." if not parsed.port + else None + ) + if problem: + raise _spotify_err(f"Spotify PKCE redirect_uri {problem}", "spotify_redirect_invalid") return host, parsed.port, parsed.path or "/" def _make_spotify_callback_handler(expected_path: str) -> tuple[type[BaseHTTPRequestHandler], dict[str, Any]]: - result: dict[str, Any] = { - "code": None, - "state": None, - "error": None, - "error_description": None, - } + result: dict[str, Any] = {"code": None, "state": None, "error": None, "error_description": None} class _SpotifyCallbackHandler(BaseHTTPRequestHandler): def do_GET(self) -> None: # noqa: N802 @@ -189,19 +160,13 @@ def _make_spotify_callback_handler(expected_path: str) -> tuple[type[BaseHTTPReq return params = parse_qs(parsed.query) - result["code"] = params.get("code", [None])[0] - result["state"] = params.get("state", [None])[0] - result["error"] = params.get("error", [None])[0] - result["error_description"] = params.get("error_description", [None])[0] + for key in result: + result[key] = params.get(key, [None])[0] self.send_response(200) self.send_header("Content-Type", "text/html; charset=utf-8") self.end_headers() - if result["error"]: - body = "

Spotify authorization failed.

You can close this tab." - else: - body = "

Spotify authorization received.

You can close this tab." - self.wfile.write(body.encode("utf-8")) + self.wfile.write(_CALLBACK_HTML.format("failed" if result["error"] else "received").encode("utf-8")) def log_message(self, format: str, *args: Any) -> None: # noqa: A003 return @@ -209,11 +174,7 @@ def _make_spotify_callback_handler(expected_path: str) -> tuple[type[BaseHTTPReq return _SpotifyCallbackHandler, result -def _spotify_wait_for_callback( - redirect_uri: str, - *, - timeout_seconds: float = 180.0, -) -> dict[str, Any]: +def _spotify_wait_for_callback(redirect_uri: str, *, timeout_seconds: float = 180.0) -> dict[str, Any]: host, port, path = _spotify_validate_redirect_uri(redirect_uri) handler_cls, result = _make_spotify_callback_handler(path) @@ -224,8 +185,7 @@ def _spotify_wait_for_callback( server = _ReuseHTTPServer((host, port), handler_cls) except OSError as exc: raise _spotify_err( - f"Could not bind Spotify callback server on {host}:{port}: {exc}", - "spotify_callback_bind_failed", + f"Could not bind Spotify callback server on {host}:{port}: {exc}", "spotify_callback_bind_failed", ) from exc thread = threading.Thread(target=server.serve_forever, kwargs={"poll_interval": 0.1}, daemon=True) @@ -240,10 +200,7 @@ def _spotify_wait_for_callback( server.shutdown() server.server_close() thread.join(timeout=1.0) - raise _spotify_err( - "Spotify authorization timed out waiting for the local callback.", - "spotify_callback_timeout", - ) + raise _spotify_err("Spotify authorization timed out waiting for the local callback.", "spotify_callback_timeout") def _spotify_token_payload_to_state( @@ -268,13 +225,9 @@ def _spotify_token_payload_to_state( "api_base_url": api_base_url, "scope": requested_scope, "granted_scope": str(token_payload.get("scope") or requested_scope).strip(), - "token_type": str(token_payload.get("token_type", "Bearer") or "Bearer").strip() or "Bearer", - "access_token": str(token_payload.get("access_token", "") or "").strip(), - "refresh_token": str( - token_payload.get("refresh_token") - or state.get("refresh_token") - or "" - ).strip(), + "token_type": _clean(token_payload.get("token_type", "Bearer") or "Bearer") or "Bearer", + "access_token": _clean(token_payload.get("access_token")), + "refresh_token": _clean(token_payload.get("refresh_token") or state.get("refresh_token")), "obtained_at": now.isoformat(), "expires_at": expires_at.isoformat(), "expires_in": expires_in, @@ -309,12 +262,11 @@ def _spotify_token_post( if response.status_code >= 400: detail = response.text.strip() raise _spotify_err( - f"Spotify {what} failed.{failed_suffix}" - + (f" Response: {detail}" if detail else ""), + f"Spotify {what} failed.{failed_suffix}" + (f" Response: {detail}" if detail else ""), failed_code, relogin=relogin_required, ) payload = response.json() - if not isinstance(payload, dict) or not str(payload.get("access_token", "") or "").strip(): + if not isinstance(payload, dict) or not _clean(payload.get("access_token")): raise _spotify_err(invalid_message, invalid_code, relogin=relogin_required) return payload @@ -345,12 +297,8 @@ def _spotify_exchange_code_for_tokens( ) -def _refresh_spotify_oauth_state( - state: Dict[str, Any], - *, - timeout_seconds: float = 20.0, -) -> Dict[str, Any]: - refresh_token = str(state.get("refresh_token", "") or "").strip() +def _refresh_spotify_oauth_state(state: Dict[str, Any], *, timeout_seconds: float = 20.0) -> Dict[str, Any]: + refresh_token = _clean(state.get("refresh_token")) if not refresh_token: raise _spotify_err( "Spotify refresh token missing. Run `hermes auth spotify` again.", @@ -361,11 +309,7 @@ def _refresh_spotify_oauth_state( accounts_base_url = _spotify_accounts_base_url(state) payload = _spotify_token_post( accounts_base_url, - { - "grant_type": "refresh_token", - "refresh_token": refresh_token, - "client_id": client_id, - }, + {"grant_type": "refresh_token", "refresh_token": refresh_token, "client_id": client_id}, timeout_seconds=timeout_seconds, what="token refresh", failed_code="spotify_refresh_failed", @@ -398,8 +342,7 @@ def resolve_spotify_runtime_credentials( state = _load_provider_state(auth_store, "spotify") if not state: raise _spotify_err( - "Spotify is not authenticated. Run `hermes auth spotify` first.", - "spotify_auth_missing", relogin=True, + "Spotify is not authenticated. Run `hermes auth spotify` first.", "spotify_auth_missing", relogin=True, ) should_refresh = bool(force_refresh) @@ -420,7 +363,7 @@ def resolve_spotify_runtime_credentials( logger.debug("Spotify OAuth: failed to persist quarantined state: %s", _save_exc) raise - access_token = str(state.get("access_token", "") or "").strip() + access_token = _clean(state.get("access_token")) if not access_token: raise _spotify_err( "Spotify access token missing. Run `hermes auth spotify` again.", @@ -433,11 +376,11 @@ def resolve_spotify_runtime_credentials( "api_key": access_token, "token_type": str(state.get("token_type", "Bearer") or "Bearer"), "base_url": _spotify_api_base_url(state), - "scope": str(state.get("granted_scope") or state.get("scope") or "").strip(), + "scope": _clean(state.get("granted_scope") or state.get("scope")), "client_id": _spotify_client_id(state=state), "redirect_uri": _spotify_redirect_uri(state=state), "expires_at": state.get("expires_at"), - "refresh_token": str(state.get("refresh_token", "") or "").strip(), + "refresh_token": _clean(state.get("refresh_token")), } @@ -448,7 +391,7 @@ def get_spotify_auth_status() -> Dict[str, Any]: return {"logged_in": False} expires_at = state.get("expires_at") - refresh_token = str(state.get("refresh_token", "") or "").strip() + refresh_token = _clean(state.get("refresh_token")) return { "logged_in": bool(refresh_token or not _is_expiring(expires_at, 0)), "auth_type": state.get("auth_type", "oauth_pkce"), @@ -462,34 +405,27 @@ def get_spotify_auth_status() -> Dict[str, Any]: def _spotify_interactive_setup(redirect_uri_hint: str) -> str: - """Walk the user through creating a Spotify developer app, persist the resulting client_id to - ~/.hermes/.env, and return it. - """ + """Walk the user through creating a Spotify developer app; persist the client_id to ~/.hermes/.env.""" from hermes_cli.auth import _is_remote_session from hermes_cli.config import save_env_value - print() - print("=" * 70) - print("Spotify first-time setup") - print("=" * 70) - print() - print("Spotify requires every user to register their own lightweight") - print("developer app. This takes about two minutes and only has to be") - print("done once per machine.") - print() - print(f"Full guide: {SPOTIFY_DOCS_URL}") - print() - print("Steps:") - print(f" 1. Opening {SPOTIFY_DASHBOARD_URL} in your browser...") - print(" 2. Click 'Create app' and fill in:") - print(" App name: anything (e.g. hermes-agent)") - print(" Description: anything") - print(f" Redirect URI: {redirect_uri_hint}") - print(" API/SDK: Web API") - print(" 3. Agree to the terms, click Save.") - print(" 4. Open the app's Settings page and copy the Client ID.") - print(" 5. Paste it below.") - print() + print( + f"\n{'=' * 70}\nSpotify first-time setup\n{'=' * 70}\n\n" + "Spotify requires every user to register their own lightweight\n" + "developer app. This takes about two minutes and only has to be\n" + "done once per machine.\n\n" + f"Full guide: {SPOTIFY_DOCS_URL}\n\n" + "Steps:\n" + f" 1. Opening {SPOTIFY_DASHBOARD_URL} in your browser...\n" + " 2. Click 'Create app' and fill in:\n" + " App name: anything (e.g. hermes-agent)\n" + " Description: anything\n" + f" Redirect URI: {redirect_uri_hint}\n" + " API/SDK: Web API\n" + " 3. Agree to the terms, click Save.\n" + " 4. Open the app's Settings page and copy the Client ID.\n" + " 5. Paste it below.\n" + ) if not _is_remote_session(): try: @@ -506,20 +442,16 @@ def _spotify_interactive_setup(redirect_uri_hint: str) -> str: raise SystemExit("Spotify setup cancelled.") if not raw: - print() - print(f"No Client ID entered. See {SPOTIFY_DOCS_URL} for the full guide.") + print(f"\nNo Client ID entered. See {SPOTIFY_DOCS_URL} for the full guide.") raise SystemExit("Spotify setup cancelled: empty Client ID.") - # Persist so subsequent `hermes auth spotify` runs skip the wizard. + # Persist so subsequent `hermes auth spotify` runs skip the wizard. Only persist a non-default + # redirect URI, to avoid pinning users to a value the default might later change to. save_env_value("HERMES_SPOTIFY_CLIENT_ID", raw) - # Only persist the redirect URI if it's non-default, to avoid pinning - # users to a value the default might later change to. if redirect_uri_hint and redirect_uri_hint != DEFAULT_SPOTIFY_REDIRECT_URI: save_env_value("HERMES_SPOTIFY_REDIRECT_URI", redirect_uri_hint) - print() - print("Saved HERMES_SPOTIFY_CLIENT_ID to ~/.hermes/.env") - print() + print("\nSaved HERMES_SPOTIFY_CLIENT_ID to ~/.hermes/.env\n") return raw @@ -527,12 +459,10 @@ def login_spotify_command(args) -> None: from hermes_cli.auth import _auth_store_lock, _can_open_graphical_browser, _is_remote_session, _load_auth_store, _print_loopback_ssh_hint, _save_auth_store, _store_provider_state, get_provider_auth_state existing_state = get_provider_auth_state("spotify") or {} - # Interactive wizard: if no client_id is configured anywhere, walk the - # user through creating the Spotify developer app instead of crashing - # with "HERMES_SPOTIFY_CLIENT_ID is required". - explicit_client_id = getattr(args, "client_id", None) + # Interactive wizard: if no client_id is configured anywhere, walk the user through creating + # the Spotify developer app instead of crashing with "HERMES_SPOTIFY_CLIENT_ID is required". try: - client_id = _spotify_client_id(explicit_client_id, existing_state) + client_id = _spotify_client_id(getattr(args, "client_id", None), existing_state) except AuthError as exc: if getattr(exc, "code", "") != "spotify_client_id_missing": raise @@ -547,27 +477,21 @@ def login_spotify_command(args) -> None: open_browser = not getattr(args, "no_browser", False) code_verifier = _spotify_code_verifier() - code_challenge = _spotify_code_challenge(code_verifier) state_nonce = uuid.uuid4().hex authorize_url = _spotify_build_authorize_url( client_id=client_id, redirect_uri=redirect_uri, scope=scope, state=state_nonce, - code_challenge=code_challenge, + code_challenge=_spotify_code_challenge(code_verifier), accounts_base_url=accounts_base_url, ) - print("Starting Spotify PKCE login...") - print(f"Client ID: {client_id}") - print(f"Redirect URI: {redirect_uri}") - print("Make sure this redirect URI is allow-listed in your Spotify app settings.") - print() - print("Open this URL to authorize Hermes:") - print(authorize_url) - print() - print(f"Full setup guide: {SPOTIFY_DOCS_URL}") - print() + print( + f"Starting Spotify PKCE login...\nClient ID: {client_id}\nRedirect URI: {redirect_uri}\n" + "Make sure this redirect URI is allow-listed in your Spotify app settings.\n\n" + f"Open this URL to authorize Hermes:\n{authorize_url}\n\nFull setup guide: {SPOTIFY_DOCS_URL}\n" + ) _print_loopback_ssh_hint(redirect_uri, docs_url=SPOTIFY_DOCS_URL) @@ -576,18 +500,14 @@ def login_spotify_command(args) -> None: opened = webbrowser.open(authorize_url) except Exception: opened = False - if opened: - print("Browser opened for Spotify authorization.") - else: - print("Could not open the browser automatically; use the URL above.") + print( + "Browser opened for Spotify authorization." if opened + else "Could not open the browser automatically; use the URL above." + ) - callback = _spotify_wait_for_callback( - redirect_uri, - timeout_seconds=float(getattr(args, "timeout", None) or 180.0), - ) + callback = _spotify_wait_for_callback(redirect_uri, timeout_seconds=float(getattr(args, "timeout", None) or 180.0)) if callback.get("error"): - detail = callback.get("error_description") or callback["error"] - raise SystemExit(f"Spotify authorization failed: {detail}") + raise SystemExit(f"Spotify authorization failed: {callback.get('error_description') or callback['error']}") if callback.get("state") != state_nonce: raise SystemExit("Spotify authorization failed: state mismatch.") @@ -613,7 +533,7 @@ def login_spotify_command(args) -> None: _store_provider_state(auth_store, "spotify", spotify_state, set_active=False) saved_to = _save_auth_store(auth_store) - print("Spotify login successful!") - print(f" Auth state: {saved_to}") - print(" Provider state saved under providers.spotify") - print(f" Docs: {SPOTIFY_DOCS_URL}") + print( + f"Spotify login successful!\n Auth state: {saved_to}\n" + f" Provider state saved under providers.spotify\n Docs: {SPOTIFY_DOCS_URL}" + ) From fb63cc500900ef7cbc54b28d98a001808e07b509 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 20:22:30 -0700 Subject: [PATCH 03/16] =?UTF-8?q?refactor(hermes=5Fcli):=20auth=5Fminimax?= =?UTF-8?q?=20=E2=80=94=20packed=20layout,=20dict-merge=20state=20updates,?= =?UTF-8?q?=20tighter=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/auth_minimax.py | 180 ++++++++++--------------------------- 1 file changed, 47 insertions(+), 133 deletions(-) diff --git a/hermes_cli/auth_minimax.py b/hermes_cli/auth_minimax.py index 055f8a7efd..d7b9b349d4 100644 --- a/hermes_cli/auth_minimax.py +++ b/hermes_cli/auth_minimax.py @@ -15,17 +15,11 @@ import json import time import uuid from datetime import datetime, timezone -from typing import Any, Callable, Dict, Optional +from typing import Any, Callable, Dict, Optional, TYPE_CHECKING from hermes_cli.auth_constants import ( - AuthError, - MINIMAX_OAUTH_GRANT_TYPE, - MINIMAX_OAUTH_REFRESH_SKEW_SECONDS, - MINIMAX_OAUTH_SCOPE, - _FORM_JSON_HEADERS, - _minimax_err, - httpx, + AuthError, MINIMAX_OAUTH_GRANT_TYPE, MINIMAX_OAUTH_REFRESH_SKEW_SECONDS, MINIMAX_OAUTH_SCOPE, + _FORM_JSON_HEADERS, _minimax_err, httpx, ) -from typing import TYPE_CHECKING if TYPE_CHECKING: # annotation-only; the runtime import would be a cycle from hermes_cli.auth import ProviderConfig @@ -37,11 +31,7 @@ logger = logging.getLogger("hermes_cli.auth") _MINIMAX_OAUTH_ERROR_BODY_LIMIT = 16 * 1024 -def _minimax_response_error_text( - response: httpx.Response, - *, - limit: int = _MINIMAX_OAUTH_ERROR_BODY_LIMIT, -) -> str: +def _minimax_response_error_text(response: httpx.Response, *, limit: int = _MINIMAX_OAUTH_ERROR_BODY_LIMIT) -> str: """Return a bounded error body from a streamed MiniMax OAuth response.""" limit = max(0, int(limit)) chunks: list[bytes] = [] @@ -70,28 +60,15 @@ def _minimax_response_error_text( if len(raw) > limit: raw = raw[:limit] truncated = True - encoding = response.encoding or "utf-8" - text = raw.decode(encoding, errors="replace") + text = raw.decode(response.encoding or "utf-8", errors="replace") return text + ("...[truncated]" if truncated else "") finally: response.close() -def _minimax_post_form( - client: httpx.Client, - url: str, - *, - data: Dict[str, Any], - headers: Dict[str, str], -) -> httpx.Response: +def _minimax_post_form(client: httpx.Client, url: str, *, data: Dict[str, Any], headers: Dict[str, str]) -> httpx.Response: """POST a MiniMax OAuth form without eagerly reading error bodies.""" - request = client.build_request( - "POST", - url, - data=data, - headers=headers, - ) - response = client.send(request, stream=True) + response = client.send(client.build_request("POST", url, data=data, headers=headers), stream=True) if response.status_code == 200: response.read() return response @@ -101,11 +78,8 @@ def _minimax_pkce_pair() -> tuple: """Generate (code_verifier, code_challenge_S256, state) for MiniMax OAuth.""" import secrets verifier = secrets.token_urlsafe(64)[:96] - challenge = base64.urlsafe_b64encode( - hashlib.sha256(verifier.encode()).digest() - ).decode().rstrip("=") - state = secrets.token_urlsafe(16) - return verifier, challenge, state + challenge = base64.urlsafe_b64encode(hashlib.sha256(verifier.encode()).digest()).decode().rstrip("=") + return verifier, challenge, secrets.token_urlsafe(16) def _minimax_request_user_code( @@ -127,17 +101,11 @@ def _minimax_request_user_code( ) if response.status_code != 200: body = _minimax_response_error_text(response) - raise _minimax_err( - f"MiniMax OAuth authorization failed: {body or response.reason_phrase}", - "authorization_failed", - ) + raise _minimax_err(f"MiniMax OAuth authorization failed: {body or response.reason_phrase}", "authorization_failed") payload = response.json() for field in ("user_code", "verification_uri", "expired_in"): if field not in payload: - raise _minimax_err( - f"MiniMax OAuth response missing field: {field}", - "authorization_incomplete", - ) + raise _minimax_err(f"MiniMax OAuth response missing field: {field}", "authorization_incomplete") if payload.get("state") != state: raise _minimax_err("MiniMax OAuth state mismatch (possible CSRF).", "state_mismatch") return payload @@ -151,8 +119,7 @@ def _minimax_expired_in_looks_like_unix_ms(expired_in: int, *, now_ms: int) -> b def _minimax_resolve_token_expiry_unix(expired_in: int, *, now: datetime) -> float: """Return access-token expiry as unix seconds (MiniMax uses ms epoch or TTL seconds).""" raw = int(expired_in) - now_ms = int(now.timestamp() * 1000) - if _minimax_expired_in_looks_like_unix_ms(raw, now_ms=now_ms): + if _minimax_expired_in_looks_like_unix_ms(raw, now_ms=int(now.timestamp() * 1000)): return raw / 1000.0 return now.timestamp() + max(1, raw) @@ -172,8 +139,8 @@ def _minimax_poll_token( client: httpx.Client, *, portal_base_url: str, client_id: str, user_code: str, code_verifier: str, expired_in: int, interval_ms: Optional[int], ) -> Dict[str, Any]: - # OpenClaw treats expired_in as a unix-ms timestamp (Date.now() < expireTimeMs). - # Defensive parsing: if it's small enough to be a duration, treat as seconds. + # OpenClaw treats expired_in as a unix-ms timestamp; if it's small enough to be a duration, + # treat it as seconds. deadline = _minimax_resolve_token_expiry_unix(expired_in, now=datetime.now(timezone.utc)) interval = max(2.0, (interval_ms or 2000) / 1000.0) @@ -189,7 +156,6 @@ def _minimax_poll_token( }, headers=_FORM_JSON_HEADERS, ) - error_text = "" if response.status_code != 200: error_text = _minimax_response_error_text(response) try: @@ -205,16 +171,10 @@ def _minimax_poll_token( status = payload.get("status") if status == "error": - raise _minimax_err( - "MiniMax OAuth reported an error. Please try again later.", - "authorization_denied", - ) + raise _minimax_err("MiniMax OAuth reported an error. Please try again later.", "authorization_denied") if status == "success": if not all(payload.get(k) for k in ("access_token", "refresh_token", "expired_in")): - raise _minimax_err( - "MiniMax OAuth success payload missing required token fields.", - "token_incomplete", - ) + raise _minimax_err("MiniMax OAuth success payload missing required token fields.", "token_incomplete") return payload # "pending" or any other status -> keep polling time.sleep(interval) @@ -228,10 +188,7 @@ def _minimax_save_auth_state(auth_state: Dict[str, Any]) -> None: _save_active_provider_state("minimax-oauth", auth_state) -def _minimax_oauth_login( - *, region: str = "global", open_browser: bool = True, - timeout_seconds: float = 15.0, -) -> Dict[str, Any]: +def _minimax_oauth_login(*, region: str = "global", open_browser: bool = True, timeout_seconds: float = 15.0) -> Dict[str, Any]: """Run MiniMax OAuth flow, persist tokens, return auth state dict.""" from hermes_cli.auth import PROVIDER_REGISTRY, _can_open_graphical_browser, _is_remote_session, _minimax_pkce_pair, _minimax_request_user_code, _minimax_save_auth_state, _print_device_code_instructions pconfig = PROVIDER_REGISTRY["minimax-oauth"] @@ -250,33 +207,24 @@ def _minimax_oauth_login( print(f"Starting Hermes login via MiniMax ({region}) OAuth...") print(f"Portal: {portal_base_url}") - with httpx.Client(timeout=httpx.Timeout(timeout_seconds), - headers={"Accept": "application/json"}, + with httpx.Client(timeout=httpx.Timeout(timeout_seconds), headers={"Accept": "application/json"}, follow_redirects=True) as client: code_data = _minimax_request_user_code( - client, portal_base_url=portal_base_url, - client_id=pconfig.client_id, - code_challenge=challenge, state=state, + client, portal_base_url=portal_base_url, client_id=pconfig.client_id, code_challenge=challenge, state=state, ) - verification_url = str(code_data["verification_uri"]) - user_code = str(code_data["user_code"]) - _print_device_code_instructions( - verification_url, - user_code, + str(code_data["verification_uri"]), str(code_data["user_code"]), open_browser=open_browser and _can_open_graphical_browser(), ) interval_raw = code_data.get("interval") - interval_ms = int(interval_raw) if interval_raw is not None else None print("Waiting for approval...") token_data = _minimax_poll_token( - client, portal_base_url=portal_base_url, - client_id=pconfig.client_id, - user_code=user_code, code_verifier=verifier, + client, portal_base_url=portal_base_url, client_id=pconfig.client_id, + user_code=str(code_data["user_code"]), code_verifier=verifier, expired_in=int(code_data["expired_in"]), - interval_ms=interval_ms, + interval_ms=int(interval_raw) if interval_raw is not None else None, ) auth_state = { @@ -300,73 +248,50 @@ def _minimax_oauth_login( return auth_state -def _refresh_minimax_oauth_state( - state: Dict[str, Any], *, timeout_seconds: float = 15.0, - force: bool = False, -) -> Dict[str, Any]: +def _refresh_minimax_oauth_state(state: Dict[str, Any], *, timeout_seconds: float = 15.0, force: bool = False) -> Dict[str, Any]: """Refresh MiniMax OAuth access token if close to expiry (or forced).""" from hermes_cli.auth import _minimax_save_auth_state if not state.get("refresh_token"): - raise _minimax_err( - "MiniMax OAuth state has no refresh_token; please re-login.", - "no_refresh_token", relogin=True, - ) + raise _minimax_err("MiniMax OAuth state has no refresh_token; please re-login.", "no_refresh_token", relogin=True) try: expires_at = datetime.fromisoformat(state.get("expires_at", "")).timestamp() except Exception: expires_at = 0.0 - now = time.time() - if not force and (expires_at - now) > MINIMAX_OAUTH_REFRESH_SKEW_SECONDS: + if not force and (expires_at - time.time()) > MINIMAX_OAUTH_REFRESH_SKEW_SECONDS: return state - portal_base_url = state["portal_base_url"] - with httpx.Client(timeout=httpx.Timeout(timeout_seconds), - follow_redirects=True) as client: + with httpx.Client(timeout=httpx.Timeout(timeout_seconds), follow_redirects=True) as client: response = _minimax_post_form( client, - f"{portal_base_url}/oauth/token", - data={ - "grant_type": "refresh_token", - "client_id": state["client_id"], - "refresh_token": state["refresh_token"], - }, + f"{state['portal_base_url']}/oauth/token", + data={"grant_type": "refresh_token", "client_id": state["client_id"], "refresh_token": state["refresh_token"]}, headers=_FORM_JSON_HEADERS, ) - # The non-200 branch reads a STREAMED body, so it must run while - # the client is still open — iter_bytes() after the client context - # closes raises (StreamClosed). The 200 path was already read by + # The non-200 branch reads a STREAMED body, so it must run while the client is still open + # (iter_bytes() after close raises StreamClosed). The 200 path was already read by # _minimax_post_form, so response.json() below is safe outside. if response.status_code != 200: body = _minimax_response_error_text(response) body_lower = body.lower() - relogin = any(m in body_lower for m in - ("invalid_grant", "refresh_token_reused", "invalid_refresh_token")) + relogin = any(m in body_lower for m in ("invalid_grant", "refresh_token_reused", "invalid_refresh_token")) raise _minimax_err( - f"MiniMax OAuth refresh failed: {body or response.reason_phrase}", - "refresh_failed", relogin=relogin, + f"MiniMax OAuth refresh failed: {body or response.reason_phrase}", "refresh_failed", relogin=relogin, ) payload = response.json() if payload.get("status") != "success": - raise _minimax_err( - "MiniMax OAuth refresh did not return success.", - "refresh_failed", relogin=True, - ) - new_state = dict(state) - new_state.update({ + raise _minimax_err("MiniMax OAuth refresh did not return success.", "refresh_failed", relogin=True) + new_state = { + **state, "access_token": payload["access_token"], "refresh_token": payload.get("refresh_token", state["refresh_token"]), **_minimax_expiry_fields(payload["expired_in"]), - }) + } _minimax_save_auth_state(new_state) return new_state def _minimax_oauth_quarantine_on_terminal_refresh(state: Dict[str, Any], exc: AuthError) -> None: - """Wipe dead tokens from auth.json after a terminal refresh failure. - - Shared by the eager-resolve path and the lazy per-request token provider. Mirrors the - Nous / xAI / Codex quarantine pattern so subsequent calls fail fast without a network retry. - """ + """Wipe dead tokens from auth.json after a terminal refresh failure (fail fast, no network retry).""" from hermes_cli.auth import _minimax_save_auth_state, _quarantine_flat_oauth_state if not (exc.relogin_required and state.get("refresh_token")): return @@ -383,9 +308,7 @@ def _minimax_fresh_state() -> Dict[str, Any]: state = get_provider_auth_state("minimax-oauth") if not state or not state.get("access_token"): raise _minimax_err( - "Not logged into MiniMax OAuth. Run `hermes model` and select " - "MiniMax (OAuth).", - "not_logged_in", relogin=True, + "Not logged into MiniMax OAuth. Run `hermes model` and select MiniMax (OAuth).", "not_logged_in", relogin=True, ) try: return _refresh_minimax_oauth_state(state) @@ -398,17 +321,13 @@ def build_minimax_oauth_token_provider() -> Callable[[], str]: """Return a zero-arg callable that yields a fresh MiniMax access token. The Anthropic SDK caches ``api_key`` as a static string at construction time, so a session that - resolves credentials once at startup will keep sending the same bearer until MiniMax's server - returns 401 — typically ~15 minutes in, because MiniMax issues short-lived access tokens. + resolves credentials once at startup would keep sending the same bearer until MiniMax returns + 401 — typically ~15 minutes in, because MiniMax issues short-lived access tokens. """ def _provide() -> str: - state = _minimax_fresh_state() - token = state.get("access_token") + token = _minimax_fresh_state().get("access_token") if not token: - raise _minimax_err( - "MiniMax OAuth state has no access_token after refresh.", - "no_access_token", relogin=True, - ) + raise _minimax_err("MiniMax OAuth state has no access_token after refresh.", "no_access_token", relogin=True) return token return _provide @@ -424,13 +343,9 @@ def resolve_minimax_oauth_runtime_credentials( like ``hermes status`` that just want to know whether a valid token exists right now. """ state = _minimax_fresh_state() - if as_token_provider: - api_key: Any = build_minimax_oauth_token_provider() - else: - api_key = state["access_token"] return { "provider": "minimax-oauth", - "api_key": api_key, + "api_key": build_minimax_oauth_token_provider() if as_token_provider else state["access_token"], "base_url": state["inference_base_url"].rstrip("/"), "source": "oauth", } @@ -439,12 +354,11 @@ def resolve_minimax_oauth_runtime_credentials( def _login_minimax_oauth(args, pconfig: ProviderConfig) -> None: """CLI entry for MiniMax OAuth login.""" from hermes_cli.auth import format_auth_error - region = getattr(args, "region", None) or "global" - open_browser = not getattr(args, "no_browser", False) - timeout = getattr(args, "timeout", None) or 15.0 try: _minimax_oauth_login( - region=region, open_browser=open_browser, timeout_seconds=timeout, + region=getattr(args, "region", None) or "global", + open_browser=not getattr(args, "no_browser", False), + timeout_seconds=getattr(args, "timeout", None) or 15.0, ) except AuthError as exc: print(format_auth_error(exc)) From 323c0d88ebd66ccf930640210a81ad2338973ef7 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 20:27:47 -0700 Subject: [PATCH 04/16] =?UTF-8?q?refactor(hermes=5Fcli):=20auth=5Fmodel=5F?= =?UTF-8?q?picker=20=E2=80=94=20shared=20labels,=20dedupe=20custom-entry?= =?UTF-8?q?=20path,=20packed=20layout?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/auth_model_picker.py | 214 +++++++++++--------------------- 1 file changed, 75 insertions(+), 139 deletions(-) diff --git a/hermes_cli/auth_model_picker.py b/hermes_cli/auth_model_picker.py index 669cb120cc..2a9497b046 100644 --- a/hermes_cli/auth_model_picker.py +++ b/hermes_cli/auth_model_picker.py @@ -16,6 +16,10 @@ from hermes_cli.auth_constants import DEFAULT_NOUS_PORTAL_URL # Log-record parity with the origin module (caplog tests pin "hermes_cli.auth"). logger = logging.getLogger("hermes_cli.auth") +_CUSTOM_LABEL = "Enter custom model name" +_SKIP_LABEL = "Skip (keep current)" +_CURRENT_SUFFIX = " ← currently in use" + def _confirm_selection_guards( model_id: str, @@ -25,23 +29,15 @@ def _confirm_selection_guards( api_key: str = "", include_kinds: Optional[List[str]] = None, ) -> bool: - """Prompt before saving a model that trips any selection guard. + """Prompt before saving a model that trips any selection guard (cost, data-policy, ...). - Runs the unified guard registry (cost, data-policy, future guards) and shows one [y/N] confirm - listing every warning that fired. Returns True to proceed, False to cancel. + Shows one [y/N] confirm listing every warning that fired. Returns True to proceed. """ try: - from hermes_cli.model_selection_guards import ( - combined_message, - selection_warnings, - ) + from hermes_cli.model_selection_guards import combined_message, selection_warnings warnings = selection_warnings( - model_id, - provider=provider, - base_url=base_url, - api_key=api_key, - include_kinds=include_kinds, + model_id, provider=provider, base_url=base_url, api_key=api_key, include_kinds=include_kinds, ) except Exception: warnings = [] @@ -81,11 +77,7 @@ class _ModelPickerRows: self.has_pricing = bool(pricing and any(pricing.get(m) for m in all_models)) # Leave room for a leading "★ " on sale rows (Nous only). name_pad = 3 if sale_chrome else 2 - self.name_col = ( - max((len(m) for m in all_models), default=0) + name_pad - if self.has_pricing - else 0 - ) + self.name_col = max((len(m) for m in all_models), default=0) + name_pad if self.has_pricing else 0 # (inp, out, cache, pct|None, was_inp, was_out) self._price_cache: dict[str, tuple[str, str, str, int | None, str, str]] = {} self.price_col = 3 # minimum width @@ -94,10 +86,15 @@ class _ModelPickerRows: self.any_on_sale = False if not self.has_pricing: return + + def _was(raw: str) -> str: + return _format_price_per_mtok(raw) if raw != "" else "?" + for mid in all_models: p = pricing.get(mid) # type: ignore[union-attr] pct: int | None = None was_inp = was_out = "" + inp, out, cache = "", "", "" if p: inp = _format_price_per_mtok(p.get("prompt", "")) out = _format_price_per_mtok(p.get("completion", "")) @@ -105,33 +102,14 @@ class _ModelPickerRows: cache = _format_price_per_mtok(cache_read) if cache_read else "" if cache: self.has_cache = True - if sale_chrome: - sale = compute_sale_discount( - p.get("prompt", ""), - p.get("completion", ""), - p.get("original"), - ) - if sale is not None: - self.any_on_sale = True - pct, was_prompt_raw, was_out_raw = sale - # Natively-free models (no gateway original) carry - # empty was_* raws — leave them empty so the row - # shows bare "-100%" with no "was ?/?" suffix. - if was_prompt_raw == "" and was_out_raw == "": - was_inp = was_out = "" - else: - was_inp = ( - _format_price_per_mtok(was_prompt_raw) - if was_prompt_raw != "" - else "?" - ) - was_out = ( - _format_price_per_mtok(was_out_raw) - if was_out_raw != "" - else "?" - ) - else: - inp, out, cache = "", "", "" + sale = compute_sale_discount(p.get("prompt", ""), p.get("completion", ""), p.get("original")) if sale_chrome else None + if sale is not None: + self.any_on_sale = True + pct, was_prompt_raw, was_out_raw = sale + # Natively-free models (no gateway original) carry empty was_* raws — leave + # them empty so the row shows bare "-100%" with no "was ?/?" suffix. + if was_prompt_raw != "" or was_out_raw != "": + was_inp, was_out = _was(was_prompt_raw), _was(was_out_raw) self._price_cache[mid] = (inp, out, cache, pct, was_inp, was_out) self.price_col = max(self.price_col, len(inp), len(out)) self.cache_col = max(self.cache_col, len(cache)) @@ -140,37 +118,27 @@ class _ModelPickerRows: def segments(self, mid: str) -> list[tuple[str, str | None]]: """Build a rich radiolist row: yellow ★/% , dim was, plain prices.""" + current = [(_CURRENT_SUFFIX, None)] if mid == self.current_model else [] if not self.has_pricing: - segs: list[tuple[str, str | None]] = [(mid, None)] - if mid == self.current_model: - segs.append((" ← currently in use", None)) - return segs + return [(mid, None), *current] - inp, out, cache, pct, was_inp, was_out = self._price_cache.get( - mid, ("", "", "", None, "", "") - ) + inp, out, cache, pct, was_inp, was_out = self._price_cache.get(mid, ("", "", "", None, "", "")) on_sale = pct is not None # Reserve 2 columns for "★ " so sale and non-sale names share alignment. - star_w = 2 if on_sale: - name_segs: list[tuple[str, str | None]] = [ - ("★ ", "yellow"), - (f"{mid:<{self.name_col - star_w}}", None), - ] + segs: list[tuple[str, str | None]] = [("★ ", "yellow"), (f"{mid:<{self.name_col - 2}}", None)] else: - name_segs = [(f"{mid:<{self.name_col}}", None)] + segs = [(f"{mid:<{self.name_col}}", None)] price_part = f" {inp:>{self.price_col}} {out:>{self.price_col}}" if self.has_cache: price_part += f" {cache:>{self.cache_col}}" - segs = [*name_segs, (price_part, None)] + segs.append((price_part, None)) if on_sale: segs.append((f" -{pct}%", "yellow")) if was_inp or was_out: segs.append((f" was {was_inp}/{was_out}", "dim")) - if mid == self.current_model: - segs.append((" ← currently in use", None)) - return segs + return segs + current def label(self, mid: str) -> str: return "".join(text for text, _style in self.segments(mid)) @@ -179,15 +147,13 @@ class _ModelPickerRows: """``Select default model:`` plus an aligned pricing header hint when priced.""" title = "Select default model:" if self.has_pricing: - # Align the header with the model column. - # Each choice is " {label}" (2 spaces) and we prepend - # a 3-char cursor region ("-> " or " "), so content starts at col 5. + # Each choice is " {label}" (2 spaces) plus a 3-char cursor region ("-> " or " "), + # so content starts at col 5. pad = " " * 5 header = f"\n{pad}{'':>{self.name_col}} {'In':>{self.price_col}} {'Out':>{self.price_col}}" if self.has_cache: header += f" {'Cache':>{self.cache_col}}" - # Legend lives on the column-header line so it reads as a key - # (★ = on sale), not a fake menu row. + # Legend lives on the column-header line so it reads as a key, not a fake menu row. title += header + " $/Mtok" if self.any_on_sale: title += " ★ = on sale" @@ -213,113 +179,91 @@ def _prompt_model_selection( from hermes_cli.cli_output import line_input _unavailable = unavailable_models or [] - # Sale chrome (★ / -N% / was) is Nous Portal-only — never for OpenRouter - # or other providers even if pricing.original is somehow present. + # Sale chrome (★ / -N% / was) is Nous Portal-only — never for OpenRouter or other providers + # even if pricing.original is somehow present. sale_chrome = (confirm_provider or "").strip().lower() == "nous" def _confirmed_selection(mid: str) -> Optional[str]: if not mid: return None - # Unified guard registry (hermes_cli.model_selection_guards): the cost - # guard only runs when a provider is known (pricing lookups need one); - # id-keyed guards like the data-policy guard always run — they must - # fire even via a custom endpoint or gateway. - _kinds = None if confirm_provider else ["data_policy"] - if not _confirm_selection_guards( - mid, - provider=confirm_provider, - base_url=confirm_base_url, - api_key=confirm_api_key, - include_kinds=_kinds, - ): + # The cost guard only runs when a provider is known (pricing lookups need one); id-keyed + # guards like the data-policy guard always run — even via a custom endpoint or gateway. + ok = _confirm_selection_guards( + mid, provider=confirm_provider, base_url=confirm_base_url, api_key=confirm_api_key, + include_kinds=None if confirm_provider else ["data_policy"], + ) + return mid if ok else None + + def _custom_selection() -> Optional[str]: + try: + custom = line_input("Enter model name: ").strip() + except (EOFError, KeyboardInterrupt): return None - return mid + return _confirmed_selection(custom) if custom else None # Reorder: current model first, then the rest (deduplicated) - ordered = [] - if current_model and current_model in model_ids: - ordered.append(current_model) - for mid in model_ids: - if mid not in ordered: - ordered.append(mid) + ordered = list(dict.fromkeys( + ([current_model] if current_model and current_model in model_ids else []) + list(model_ids) + )) # All models for column-width computation (selectable + unavailable) - rows = _ModelPickerRows( - list(ordered) + list(_unavailable), pricing, - current_model=current_model, sale_chrome=sale_chrome, - ) + rows = _ModelPickerRows(ordered + list(_unavailable), pricing, current_model=current_model, sale_chrome=sale_chrome) _DIM = "\033[2m" _RESET = "\033[0m" - # Default cursor on the current model (index 0 if it was reordered to top) - default_idx = 0 menu_title = rows.menu_title() _upgrade_url = (portal_url or DEFAULT_NOUS_PORTAL_URL).rstrip("/") + n = len(ordered) # Try arrow-key menu first, fall back to number input. try: from hermes_cli.curses_ui import curses_radiolist - choices = [rows.segments(mid) for mid in ordered] - choices.append("Enter custom model name") - choices.append("Skip (keep current)") + choices = [rows.segments(mid) for mid in ordered] + [_CUSTOM_LABEL, _SKIP_LABEL] unavailable_footer = unavailable_message.strip() if not unavailable_footer and _unavailable: unavailable_footer = f"Upgrade at {_upgrade_url} for paid models" - # The pricing column header (and any unavailable-models block) is shown - # as a multi-line description above the list so it survives the curses - # screen clear. menu_title already embeds the aligned price header. + # The pricing column header (and any unavailable-models block) is shown as a multi-line + # description above the list so it survives the curses screen clear. menu_title already + # embeds the aligned price header; keep only the header/legend portion. desc_lines: list[str] = [] if rows.has_pricing: - # menu_title is "Select default model:\n
$/Mtok\n…" - # Keep only the header/legend portion for the description. header_part = menu_title.split("\n", 1) if len(header_part) > 1: desc_lines.extend(header_part[1].splitlines()) if _unavailable: - for mid in _unavailable: - desc_lines.append(f" {rows.label(mid)}") + desc_lines.extend(f" {rows.label(mid)}" for mid in _unavailable) desc_lines.append(f" ── {unavailable_footer} ──") - description = "\n".join(desc_lines) if desc_lines else None - # Search haystacks keep pricing labels visible while adding aliases - # for brand-less wire ids (e.g. Kimi Coding `k3` ↔ query "kimi"). + # Search haystacks keep pricing labels visible while adding aliases for brand-less wire + # ids (e.g. Kimi Coding `k3` ↔ query "kimi"). model_search_text always starts with the + # wire id; only append when aliases add tokens beyond the bare id already in the label. from hermes_cli.model_search import model_search_text model_search_labels = [] for mid in ordered: - label = rows.label(mid) - haystack = model_search_text(mid) - # model_search_text always starts with the wire id; only append when - # aliases add tokens beyond the bare id already in the label. - model_search_labels.append( - label if haystack == mid else f"{label} {haystack}" - ) - model_search_labels.append("Enter custom model name") - model_search_labels.append("Skip (keep current)") + label, haystack = rows.label(mid), model_search_text(mid) + model_search_labels.append(label if haystack == mid else f"{label} {haystack}") + model_search_labels += [_CUSTOM_LABEL, _SKIP_LABEL] idx = curses_radiolist( "Select default model:", choices, - selected=default_idx, + selected=0, # cursor on the current model (index 0 if it was reordered to top) cancel_returns=-1, - description=description, + description="\n".join(desc_lines) if desc_lines else None, searchable=True, search_labels=model_search_labels, ) if idx < 0: return None print() - if idx < len(ordered): + if idx < n: return _confirmed_selection(ordered[idx]) - elif idx == len(ordered): - try: - custom = line_input("Enter model name: ").strip() - except (EOFError, KeyboardInterrupt): - return None - return _confirmed_selection(custom) if custom else None + if idx == n: + return _custom_selection() return None except (ImportError, NotImplementedError, OSError, subprocess.SubprocessError): pass @@ -329,16 +273,12 @@ def _prompt_model_selection( from hermes_cli.colors import Colors, color for line in menu_title.splitlines(): - if "★" in line: - print(line.replace("★", color("★", Colors.YELLOW), 1)) - else: - print(line) - num_width = len(str(len(ordered) + 2)) + print(line.replace("★", color("★", Colors.YELLOW), 1) if "★" in line else line) + num_width = len(str(n + 2)) for i, mid in enumerate(ordered, 1): print(f" {i:>{num_width}}. {format_radio_item_ansi(rows.segments(mid))}") - n = len(ordered) - print(f" {n + 1:>{num_width}}. Enter custom model name") - print(f" {n + 2:>{num_width}}. Skip (keep current)") + print(f" {n + 1:>{num_width}}. {_CUSTOM_LABEL}") + print(f" {n + 2:>{num_width}}. {_SKIP_LABEL}") if _unavailable: unavailable_footer = unavailable_message.strip() or ( @@ -358,10 +298,10 @@ def _prompt_model_selection( idx = int(choice) if 1 <= idx <= n: return _confirmed_selection(ordered[idx - 1]) - elif idx == n + 1: + if idx == n + 1: custom = line_input("Enter model name: ").strip() return _confirmed_selection(custom) if custom else None - elif idx == n + 2: + if idx == n + 2: return None print(f"Please enter 1-{n + 2}") except ValueError: @@ -371,11 +311,7 @@ def _prompt_model_selection( def _save_model_choice(model_id: str) -> None: - """Save the selected model to config.yaml (single source of truth). - - The model is stored in config.yaml only — NOT in .env. This avoids conflicts in multi-agent - setups where env vars would stomp each other. - """ + """Save the selected model to config.yaml only — NOT .env, which would stomp in multi-agent setups.""" from hermes_cli.config import save_config, load_config config = load_config() From 77d6e5700d38f6d101b9804ea2964f6c42b38099 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 20:31:18 -0700 Subject: [PATCH 05/16] =?UTF-8?q?refactor(hermes=5Fcli):=20auth=5Fzai=5Fki?= =?UTF-8?q?mi/auth=5Fqwen=20=E2=80=94=20priority-walk=20helper,=20shared?= =?UTF-8?q?=20probe=20model=20list,=20packed=20layout?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/auth_qwen.py | 75 ++++--------- hermes_cli/auth_zai_kimi.py | 208 +++++++++++++----------------------- 2 files changed, 95 insertions(+), 188 deletions(-) diff --git a/hermes_cli/auth_qwen.py b/hermes_cli/auth_qwen.py index 69235b3296..1f82e3daab 100644 --- a/hermes_cli/auth_qwen.py +++ b/hermes_cli/auth_qwen.py @@ -15,19 +15,15 @@ import time from pathlib import Path from typing import Any, Dict from hermes_cli.auth_constants import ( - AuthError, - DEFAULT_QWEN_BASE_URL, - QWEN_ACCESS_TOKEN_REFRESH_SKEW_SECONDS, - QWEN_OAUTH_CLIENT_ID, - QWEN_OAUTH_TOKEN_URL, - _FORM_JSON_HEADERS, - _qwen_err, - httpx, + AuthError, DEFAULT_QWEN_BASE_URL, QWEN_ACCESS_TOKEN_REFRESH_SKEW_SECONDS, QWEN_OAUTH_CLIENT_ID, + QWEN_OAUTH_TOKEN_URL, _FORM_JSON_HEADERS, _qwen_err, httpx, ) # Log-record parity with the origin module (caplog tests pin "hermes_cli.auth"). logger = logging.getLogger("hermes_cli.auth") +_RERUN = "Re-run 'qwen auth qwen-oauth'." + def _qwen_cli_auth_path() -> Path: return Path.home() / ".qwen" / "oauth_creds.json" @@ -37,16 +33,12 @@ def _read_qwen_cli_tokens() -> Dict[str, Any]: from hermes_cli.auth import _qwen_cli_auth_path auth_path = _qwen_cli_auth_path() if not auth_path.exists(): - raise _qwen_err( - "Qwen CLI credentials not found. Run 'qwen auth qwen-oauth' first.", - "qwen_auth_missing", - ) + raise _qwen_err("Qwen CLI credentials not found. Run 'qwen auth qwen-oauth' first.", "qwen_auth_missing") try: data = json.loads(auth_path.read_text(encoding="utf-8")) except Exception as exc: raise _qwen_err( - f"Failed to read Qwen CLI credentials from {auth_path}: {exc}", - "qwen_auth_read_failed", + f"Failed to read Qwen CLI credentials from {auth_path}: {exc}", "qwen_auth_read_failed", ) from exc if not isinstance(data, dict): raise _qwen_err(f"Invalid Qwen CLI credentials in {auth_path}.", "qwen_auth_invalid") @@ -71,20 +63,13 @@ def _qwen_access_token_is_expiring(expiry_date_ms: Any, skew_seconds: int = QWEN def _refresh_qwen_cli_tokens(tokens: Dict[str, Any], timeout_seconds: float = 20.0) -> Dict[str, Any]: refresh_token = str(tokens.get("refresh_token", "") or "").strip() if not refresh_token: - raise _qwen_err( - "Qwen OAuth refresh token missing. Re-run 'qwen auth qwen-oauth'.", - "qwen_refresh_token_missing", - ) + raise _qwen_err(f"Qwen OAuth refresh token missing. {_RERUN}", "qwen_refresh_token_missing") try: response = httpx.post( QWEN_OAUTH_TOKEN_URL, headers=_FORM_JSON_HEADERS, - data={ - "grant_type": "refresh_token", - "refresh_token": refresh_token, - "client_id": QWEN_OAUTH_CLIENT_ID, - }, + data={"grant_type": "refresh_token", "refresh_token": refresh_token, "client_id": QWEN_OAUTH_CLIENT_ID}, timeout=timeout_seconds, ) except Exception as exc: @@ -93,28 +78,19 @@ def _refresh_qwen_cli_tokens(tokens: Dict[str, Any], timeout_seconds: float = 20 if response.status_code >= 400: body = response.text.strip() raise _qwen_err( - "Qwen OAuth refresh failed. Re-run 'qwen auth qwen-oauth'." - + (f" Response: {body}" if body else ""), - "qwen_refresh_failed", + f"Qwen OAuth refresh failed. {_RERUN}" + (f" Response: {body}" if body else ""), "qwen_refresh_failed", ) try: payload = response.json() except Exception as exc: - raise _qwen_err( - f"Qwen OAuth refresh returned invalid JSON: {exc}", - "qwen_refresh_invalid_json", - ) from exc + raise _qwen_err(f"Qwen OAuth refresh returned invalid JSON: {exc}", "qwen_refresh_invalid_json") from exc if not isinstance(payload, dict) or not str(payload.get("access_token", "") or "").strip(): - raise _qwen_err( - "Qwen OAuth refresh response missing access_token.", - "qwen_refresh_invalid_response", - ) + raise _qwen_err("Qwen OAuth refresh response missing access_token.", "qwen_refresh_invalid_response") - expires_in = payload.get("expires_in") try: - expires_in_seconds = int(expires_in) + expires_in_seconds = int(payload.get("expires_in")) except Exception: expires_in_seconds = 6 * 60 * 60 @@ -139,9 +115,7 @@ def _mark_qwen_oauth_active(creds: Dict[str, Any]) -> None: from hermes_cli.auth import _auth_store_lock, _load_auth_store, _save_auth_store, _save_provider_state with _auth_store_lock(): auth_store = _load_auth_store() - state: Dict[str, Any] = {} - if creds.get("base_url"): - state["base_url"] = str(creds["base_url"]) + state: Dict[str, Any] = {"base_url": str(creds["base_url"])} if creds.get("base_url") else {} _save_provider_state(auth_store, "qwen-oauth", state) _save_auth_store(auth_store) @@ -154,23 +128,18 @@ def resolve_qwen_runtime_credentials( ) -> Dict[str, Any]: from hermes_cli.auth import _qwen_cli_auth_path, _refresh_qwen_cli_tokens tokens = _read_qwen_cli_tokens() - access_token = str(tokens.get("access_token", "") or "").strip() should_refresh = bool(force_refresh) if not should_refresh and refresh_if_expiring: should_refresh = _qwen_access_token_is_expiring(tokens.get("expiry_date"), refresh_skew_seconds) if should_refresh: tokens = _refresh_qwen_cli_tokens(tokens) - access_token = str(tokens.get("access_token", "") or "").strip() + access_token = str(tokens.get("access_token", "") or "").strip() if not access_token: - raise _qwen_err( - "Qwen OAuth access token missing. Re-run 'qwen auth qwen-oauth'.", - "qwen_access_token_missing", - ) + raise _qwen_err(f"Qwen OAuth access token missing. {_RERUN}", "qwen_access_token_missing") - base_url = os.getenv("HERMES_QWEN_BASE_URL", "").strip().rstrip("/") or DEFAULT_QWEN_BASE_URL return { "provider": "qwen-oauth", - "base_url": base_url, + "base_url": os.getenv("HERMES_QWEN_BASE_URL", "").strip().rstrip("/") or DEFAULT_QWEN_BASE_URL, "api_key": access_token, "source": "qwen-cli", "expires_at_ms": tokens.get("expiry_date"), @@ -182,9 +151,9 @@ def get_qwen_auth_status() -> Dict[str, Any]: from hermes_cli.auth import _qwen_cli_auth_path, resolve_qwen_runtime_credentials auth_path = _qwen_cli_auth_path() try: - # Validate the runtime credentials, including refresh when the cached - # CLI token is expired. Otherwise stale tokens show up as "logged in" - # and `hermes model` walks users into a broken Qwen setup flow. + # Validate the runtime credentials, including refresh when the cached CLI token is expired; + # otherwise stale tokens show up as "logged in" and `hermes model` walks users into a + # broken Qwen setup flow. creds = resolve_qwen_runtime_credentials(refresh_if_expiring=True) return { "logged_in": True, @@ -194,8 +163,4 @@ def get_qwen_auth_status() -> Dict[str, Any]: "expires_at_ms": creds.get("expires_at_ms"), } except AuthError as exc: - return { - "logged_in": False, - "auth_file": str(auth_path), - "error": str(exc), - } + return {"logged_in": False, "auth_file": str(auth_path), "error": str(exc)} diff --git a/hermes_cli/auth_zai_kimi.py b/hermes_cli/auth_zai_kimi.py index 5a3c78d25c..64ab2733d3 100644 --- a/hermes_cli/auth_zai_kimi.py +++ b/hermes_cli/auth_zai_kimi.py @@ -17,83 +17,50 @@ from hermes_cli.auth_constants import httpx logger = logging.getLogger("hermes_cli.auth") -# Kimi Code (kimi.com/code) issues keys prefixed "sk-kimi-" that only work -# on api.kimi.com/coding. Legacy keys from platform.moonshot.ai work on -# api.moonshot.ai/v1 (the old default). Auto-detect when user hasn't set -# KIMI_BASE_URL explicitly. -# -# Note: the base URL intentionally has NO /v1 suffix. The /coding endpoint -# speaks the Anthropic Messages protocol, and the anthropic SDK appends -# "/v1/messages" internally — so "/coding" + SDK suffix → "/coding/v1/messages" -# (the correct target). Using "/coding/v1" here would produce -# "/coding/v1/v1/messages" (a 404). +# Kimi Code (kimi.com/code) issues "sk-kimi-" keys that only work on api.kimi.com/coding; legacy +# platform.moonshot.ai keys work on api.moonshot.ai/v1 (the old default). Intentionally NO /v1 +# suffix: the /coding endpoint speaks Anthropic Messages and the SDK appends "/v1/messages" +# itself — "/coding/v1" would produce "/coding/v1/v1/messages" (a 404). KIMI_CODE_BASE_URL = "https://api.kimi.com/coding" def _resolve_kimi_base_url(api_key: str, default_url: str, env_override: str) -> str: - """Return the correct Kimi base URL based on the API key prefix. - - If the user has explicitly set KIMI_BASE_URL, that always wins. Otherwise, sk-kimi- prefixed - keys route to api.kimi.com/coding/v1. - """ + """Kimi base URL from the key prefix; an explicit KIMI_BASE_URL always wins.""" if env_override: return env_override - # No key → nothing to infer from. Return default without inspecting. - if not api_key: - return default_url - if api_key.startswith("sk-kimi-"): + if api_key and api_key.startswith("sk-kimi-"): return KIMI_CODE_BASE_URL return default_url -# Z.AI has separate billing for general vs coding plans, and global vs China -# endpoints. A key that works on one may return "Insufficient balance" on -# another. We probe at setup time and store the working endpoint. -# Each entry lists candidate models to try in order — newer coding plan accounts -# may only have access to recent models (glm-5.1, glm-5v-turbo) while older -# ones still use glm-4.7. +# Z.AI bills general vs coding plans, and global vs China endpoints, separately; a key that works +# on one may return "Insufficient balance" on another, so we probe at setup time and store the +# working endpoint. Each entry lists candidate models to try in order — newer coding-plan accounts +# may only have access to recent models while older ones still use glm-4.7. +_ZAI_CODING_PROBE_MODELS = ["glm-5.3", "glm-5.3-flash", "glm-5.2", "glm-5.1", "glm-5v-turbo", "glm-4.7"] ZAI_ENDPOINTS = [ # (id, base_url, probe_models, label) ("global", "https://api.z.ai/api/paas/v4", ["glm-5"], "Global"), ("cn", "https://open.bigmodel.cn/api/paas/v4", ["glm-5"], "China"), - ("coding-global", "https://api.z.ai/api/coding/paas/v4", ["glm-5.3", "glm-5.3-flash", "glm-5.2", "glm-5.1", "glm-5v-turbo", "glm-4.7"], "Global (Coding Plan)"), - ("coding-cn", "https://open.bigmodel.cn/api/coding/paas/v4", ["glm-5.3", "glm-5.3-flash", "glm-5.2", "glm-5.1", "glm-5v-turbo", "glm-4.7"], "China (Coding Plan)"), + ("coding-global", "https://api.z.ai/api/coding/paas/v4", _ZAI_CODING_PROBE_MODELS, "Global (Coding Plan)"), + ("coding-cn", "https://open.bigmodel.cn/api/coding/paas/v4", _ZAI_CODING_PROBE_MODELS, "China (Coding Plan)"), ] -def _probe_single_zai_endpoint( - api_key: str, endpoint: tuple, timeout: float, -) -> Optional[Dict[str, str]]: - """Probe a single Z.AI endpoint. Returns endpoint info dict or None. - - Preserves the per-endpoint candidate-model loop: endpoints carry a ``probe_models`` LIST and - each model is tried in order until one succeeds (some plans only accept newer/older GLM slugs). - """ +def _probe_single_zai_endpoint(api_key: str, endpoint: tuple, timeout: float) -> Optional[Dict[str, str]]: + """Probe one Z.AI endpoint, trying its candidate models in order; None when none succeeds.""" ep_id, base_url, probe_models, label = endpoint for model in probe_models: try: resp = httpx.post( f"{base_url}/chat/completions", - headers={ - "Authorization": f"Bearer {api_key}", - "Content-Type": "application/json", - }, - json={ - "model": model, - "stream": False, - "max_tokens": 1, - "messages": [{"role": "user", "content": "ping"}], - }, + headers={"Authorization": f"Bearer {api_key}", "Content-Type": "application/json"}, + json={"model": model, "stream": False, "max_tokens": 1, "messages": [{"role": "user", "content": "ping"}]}, timeout=timeout, ) if resp.status_code == 200: logger.debug("Z.AI endpoint probe: %s (%s) model=%s OK", ep_id, base_url, model) - return { - "id": ep_id, - "base_url": base_url, - "model": model, - "label": label, - } + return {"id": ep_id, "base_url": base_url, "model": model, "label": label} logger.debug("Z.AI endpoint probe: %s model=%s returned %s", ep_id, model, resp.status_code) except Exception as exc: logger.debug("Z.AI endpoint probe: %s model=%s failed: %s", ep_id, model, exc) @@ -101,124 +68,99 @@ def _probe_single_zai_endpoint( def detect_zai_endpoint(api_key: str, timeout: float = 8.0) -> Optional[Dict[str, str]]: - """Probe z.ai endpoints in parallel to find one that accepts this API key. - - Returns {"id": ..., "base_url": ..., "model": ..., "label": ...} for the first working endpoint - (in ZAI_ENDPOINTS priority order), or None if all fail. For endpoints with multiple candidate - models, each worker tries its endpoint's models in order and returns the first that succeeds. - """ + """Probe z.ai endpoints in parallel; first working one in ZAI_ENDPOINTS priority order, or None.""" from concurrent.futures import ThreadPoolExecutor, as_completed - # No `with` block: a context manager would join ALL probe threads on - # exit, defeating the early return below. shutdown(wait=False) lets the - # surviving daemon-style probes drain in the background instead of - # blocking the caller on slow/unreachable endpoints. + # No `with` block: a context manager would join ALL probe threads on exit, defeating the early + # return below. shutdown(wait=False) lets surviving probes drain in the background. pool = ThreadPoolExecutor(max_workers=len(ZAI_ENDPOINTS)) try: - futures = { - pool.submit(_probe_single_zai_endpoint, api_key, ep, timeout): ep[0] - for ep in ZAI_ENDPOINTS - } + futures = {pool.submit(_probe_single_zai_endpoint, api_key, ep, timeout): ep[0] for ep in ZAI_ENDPOINTS} by_id = {ep_id: f for f, ep_id in futures.items()} results: Dict[str, Dict[str, str]] = {} + + def _first_ready(require_done: bool) -> Optional[Dict[str, str]]: + # Walk endpoints in PRIORITY order; a lower-priority success only wins once every + # higher-priority probe has finished without success. + for ep in ZAI_ENDPOINTS: + if require_done and not by_id[ep[0]].done(): + return None # a higher-priority probe is still in flight + if ep[0] in results: + return results[ep[0]] + return None + for future in as_completed(futures): - ep_id = futures[future] try: result = future.result() if result is not None: - results[ep_id] = result + results[futures[future]] = result except Exception: pass - # Early exit in PRIORITY order: walk endpoints highest-priority - # first; if one has succeeded and every higher-priority probe - # has already finished (without success), no later completion - # can win — return now instead of waiting out slow endpoints - # (main's sequential loop also stopped at first success). - for ep in ZAI_ENDPOINTS: - if not by_id[ep[0]].done(): - break # a higher-priority probe is still in flight - if ep[0] in results: - return results[ep[0]] - - # All probes finished: first match in priority order, if any. - for ep in ZAI_ENDPOINTS: - if ep[0] in results: - return results[ep[0]] - return None + winner = _first_ready(require_done=True) + if winner is not None: + return winner + return _first_ready(require_done=False) finally: pool.shutdown(wait=False) def _resolve_zai_base_url(api_key: str, default_url: str, env_override: str) -> str: - """Return the correct Z.AI base URL by probing endpoints. + """Z.AI base URL by probing endpoints; an explicit GLM_BASE_URL always wins. - If the user has explicitly set GLM_BASE_URL, that always wins. Otherwise, probe the candidate - endpoints to find one that accepts the key. The detected endpoint is cached in provider state - (auth.json) keyed on a hash of the API key so subsequent starts skip the probe. + The detected endpoint is cached in provider state (auth.json) keyed on a hash of the API key so + subsequent starts skip the probe. """ from hermes_cli.auth import _auth_store_lock, _load_auth_store, _load_provider_state, _save_auth_store, _store_provider_state, detect_zai_endpoint if env_override: return env_override - - # No API key set → don't probe (would fire N×M HTTPS requests with an - # empty Bearer token, all returning 401). This path is hit during - # auxiliary-client auto-detection when the user has no Z.AI credentials - # at all — the caller discards the result immediately, so the probe is - # pure latency for every AIAgent construction. + # No API key → don't probe (N×M HTTPS requests with an empty Bearer, all 401). Hit during + # auxiliary-client auto-detection for users with no Z.AI credentials at all — pure latency. if not api_key: return default_url - # Check provider-state cache for a previously-detected endpoint. - auth_store = _load_auth_store() - state = _load_provider_state(auth_store, "zai") or {} + key_hash = hashlib.sha256(api_key.encode()).hexdigest()[:16] + state = _load_provider_state(_load_auth_store(), "zai") or {} cached = state.get("detected_endpoint") - if isinstance(cached, dict) and cached.get("base_url"): - key_hash = cached.get("key_hash", "") - if key_hash == hashlib.sha256(api_key.encode()).hexdigest()[:16]: - logger.debug("Z.AI: using cached endpoint %s", cached["base_url"]) - return cached["base_url"] + if isinstance(cached, dict) and cached.get("base_url") and cached.get("key_hash", "") == key_hash: + logger.debug("Z.AI: using cached endpoint %s", cached["base_url"]) + return cached["base_url"] # Probe — may take up to ~8s per endpoint. detected = detect_zai_endpoint(api_key) - if detected and detected.get("base_url"): - # Persist the detection result keyed on the API key hash. - key_hash = hashlib.sha256(api_key.encode()).hexdigest()[:16] - detected_endpoint = { - "base_url": detected["base_url"], - "endpoint_id": detected.get("id", ""), - "model": detected.get("model", ""), - "label": detected.get("label", ""), - "key_hash": key_hash, - } - # Persist failure (disk full, permissions, lock timeout) must not - # break resolution — detection already succeeded; worst case the - # next start re-probes. - try: - with _auth_store_lock(): - # Reload auth_store under lock to avoid overwriting concurrent changes - auth_store = _load_auth_store() - state_under_lock = _load_provider_state(auth_store, "zai") or {} - state_under_lock["detected_endpoint"] = detected_endpoint - # set_active=False: this runs from credential-pool env seeding - # (agent/credential_pool.py) for ANY user with a Z.AI key in env, - # and caching a probe result must not flip their active provider. - _store_provider_state(auth_store, "zai", state_under_lock, set_active=False) - _save_auth_store(auth_store) - except Exception as exc: - logger.warning("Z.AI: could not persist detected endpoint (%s); will re-probe next start", exc) - logger.info("Z.AI: auto-detected endpoint %s (%s)", detected["label"], detected["base_url"]) - return detected["base_url"] + if not (detected and detected.get("base_url")): + logger.debug("Z.AI: probe failed, falling back to default %s", default_url) + return default_url - logger.debug("Z.AI: probe failed, falling back to default %s", default_url) - return default_url + detected_endpoint = { + "base_url": detected["base_url"], + "endpoint_id": detected.get("id", ""), + "model": detected.get("model", ""), + "label": detected.get("label", ""), + "key_hash": key_hash, + } + # Persist failure (disk full, permissions, lock timeout) must not break resolution — detection + # already succeeded; worst case the next start re-probes. + try: + with _auth_store_lock(): + auth_store = _load_auth_store() # reload under lock to avoid overwriting concurrent changes + state_under_lock = _load_provider_state(auth_store, "zai") or {} + state_under_lock["detected_endpoint"] = detected_endpoint + # set_active=False: this runs from credential-pool env seeding for ANY user with a Z.AI + # key in env, and caching a probe result must not flip their active provider. + _store_provider_state(auth_store, "zai", state_under_lock, set_active=False) + _save_auth_store(auth_store) + except Exception as exc: + logger.warning("Z.AI: could not persist detected endpoint (%s); will re-probe next start", exc) + logger.info("Z.AI: auto-detected endpoint %s (%s)", detected["label"], detected["base_url"]) + return detected["base_url"] def _normalize_lmstudio_runtime_base_url(base_url: str) -> str: """Return the OpenAI-compatible LM Studio runtime base URL. LM Studio's native management API lives under ``/api/v1`` while its OpenAI-compatible chat - endpoint lives under ``/v1``. Users often paste either form into ``LM_BASE_URL`` or - ``model.base_url``; normalize before the OpenAI SDK appends ``/chat/completions``. + endpoint lives under ``/v1``; users paste either form, so normalize before the SDK appends + ``/chat/completions``. """ root = str(base_url or "").strip().rstrip("/") for suffix in ("/api/v1", "/api", "/v1"): From ff36d20de0c5ab810bfae8765be8eedcf3b5bc1c Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 20:34:28 -0700 Subject: [PATCH 06/16] =?UTF-8?q?refactor(hermes=5Fcli):=20azure=5Fdetect?= =?UTF-8?q?=20=E2=80=94=20TokenProvider=20alias,=20collapsed=20except=20la?= =?UTF-8?q?dders,=20tighter=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/azure_detect.py | 201 +++++++++++-------------------------- 1 file changed, 58 insertions(+), 143 deletions(-) diff --git a/hermes_cli/azure_detect.py b/hermes_cli/azure_detect.py index d5e24c2039..179a38f92b 100644 --- a/hermes_cli/azure_detect.py +++ b/hermes_cli/azure_detect.py @@ -20,17 +20,16 @@ from hermes_cli.urllib_security import open_credentialed_url logger = logging.getLogger(__name__) +TokenProvider = Optional[Callable[[], str]] -# Default Azure OpenAI ``api-version`` to probe with. The v1 GA endpoint -# accepts requests without ``api-version`` entirely, so this is only used -# as a fallback for pre-v1 resources that still require it. +# Azure OpenAI ``api-version`` fallbacks for pre-v1 resources; the v1 GA endpoint accepts requests +# without ``api-version`` entirely, so these are only probed second. _AZURE_OPENAI_PROBE_API_VERSIONS = ( "2025-04-01-preview", "2024-10-21", # oldest GA that supports /models ) -# Default Azure Anthropic ``api-version``. Matches the value used by -# ``agent/anthropic_adapter.py`` when building the Anthropic client. +# Matches the value ``agent/anthropic_adapter.py`` uses when building the Anthropic client. _AZURE_ANTHROPIC_API_VERSION = "2025-04-15" @@ -38,37 +37,25 @@ _AZURE_ANTHROPIC_API_VERSION = "2025-04-15" class DetectionResult: """Everything auto-detection could gather from a base URL + API key.""" - #: Detected API transport: ``"chat_completions"``, - #: ``"anthropic_messages"``, or ``None`` when detection failed. + #: ``"chat_completions"``, ``"anthropic_messages"``, or ``None`` when detection failed. api_mode: Optional[str] = None - - #: Deployment / model IDs returned by ``/models`` (best effort). - #: Empty when the endpoint doesn't expose the list with an API key. + #: Deployment / model IDs returned by ``/models`` (best effort; empty when not exposed). models: list[str] = field(default_factory=list) - #: Lowercased host from the base URL (used for display messages). hostname: str = "" - - #: Human-readable reason the detector chose ``api_mode``. Useful - #: for explaining auto-detection to the user in the wizard. + #: Human-readable reason the detector chose ``api_mode`` (shown by the wizard). reason: str = "" - #: ``True`` when ``/models`` returned a valid OpenAI-shaped payload. models_probe_ok: bool = False - - #: ``True`` when the URL was determined to be an Anthropic-style - #: endpoint (from path suffix or live probe). + #: ``True`` when the URL was determined to be Anthropic-style (path suffix or live probe). is_anthropic: bool = False -def _resolve_credential(api_key: Any, - token_provider: Optional[Callable[[], str]] = None, - ) -> tuple[Optional[str], str]: - """Coerce wizard inputs into a (token, mode) pair. +def _resolve_credential(api_key: Any, token_provider: TokenProvider = None) -> tuple[Optional[str], str]: + """Coerce wizard inputs into ``(token_or_None, mode)``. - Returns ``(token_or_None, mode)`` where ``mode`` is: - ``"entra_id"`` when a callable token - provider was supplied — the returned token is a freshly minted bearer JWT, sent ONLY in - ``Authorization: Bearer``. + ``mode`` is ``"entra_id"`` when a callable token provider was supplied (the token is a freshly + minted bearer JWT, sent ONLY in ``Authorization: Bearer``), else ``"api_key"``. """ # Token-provider path (callable wins when both supplied). for provider, label in ((token_provider, "token_provider"), (api_key, "api_key callable")): @@ -79,7 +66,6 @@ def _resolve_credential(api_key: Any, except Exception as exc: logger.debug("azure_detect: %s failed: %s", label, exc) return None, "entra_id" - # API-key path. if isinstance(api_key, str) and api_key: return api_key, "api_key" return None, "api_key" @@ -91,25 +77,18 @@ def _authed_request(url: str, api_key: Any, token_provider, *, method: str = "GE token, mode = _resolve_credential(api_key, token_provider) req = urllib_request.Request(url, method=method, data=data) if token: + # Legacy broad-compat behaviour sends both headers so we land on any Azure resource. In + # entra_id mode send Bearer ONLY — api-key would log a JWT in a slot meant for static keys. if mode != "entra_id": - # Legacy broad-compat behaviour: send both headers so we land on - # any Azure resource regardless of which it accepts. req.add_header("api-key", token) - # Bearer-only in entra_id mode: do NOT also set api-key, which would - # log a JWT in a header slot intended for static keys. req.add_header("Authorization", f"Bearer {token}") req.add_header("User-Agent", "hermes-agent/azure-detect") return req -def _http_get_json(url: str, - api_key: Any, - timeout: float = 6.0, - *, - token_provider: Optional[Callable[[], str]] = None, - ) -> tuple[int, Optional[dict]]: - """GET a URL with the appropriate auth headers. Return - ``(status_code, parsed_json_or_None)``. Never raises.""" +def _http_get_json(url: str, api_key: Any, timeout: float = 6.0, *, + token_provider: TokenProvider = None) -> tuple[int, Optional[dict]]: + """GET with auth headers; return ``(status_code, parsed_json_or_None)``. Never raises.""" req = _authed_request(url, api_key, token_provider) try: with open_credentialed_url(req, timeout=timeout) as resp: @@ -134,166 +113,115 @@ def _strip_trailing_v1(url: str) -> str: def _looks_like_anthropic_path(url: str) -> bool: - """Return True when the URL's path ends in ``/anthropic`` or contains a ``/anthropic/`` segment. - Used by Azure Foundry resources that route Claude traffic through a dedicated path. - """ + """True when the path ends in ``/anthropic`` or contains a ``/anthropic/`` segment (Foundry Claude routes).""" try: - parsed = urlparse(url) - path = (parsed.path or "").lower().rstrip("/") + path = (urlparse(url).path or "").lower().rstrip("/") return path.endswith("/anthropic") or "/anthropic/" in path + "/" except Exception: return False def _extract_model_ids(payload: dict) -> list[str]: - """Extract a list of model IDs from an OpenAI-shaped ``/models`` - response. Returns ``[]`` on any shape mismatch.""" + """Model IDs from an OpenAI-shaped ``/models`` response; ``[]`` on any shape mismatch.""" data = payload.get("data") if isinstance(payload, dict) else None if not isinstance(data, list): return [] ids: list[str] = [] for item in data: - if not isinstance(item, dict): - continue - # OpenAI shape: {"id": "gpt-5.4", "object": "model", ...} - mid = item.get("id") or item.get("model") or item.get("name") - if isinstance(mid, str) and mid: - ids.append(mid) + if isinstance(item, dict): + mid = item.get("id") or item.get("model") or item.get("name") + if isinstance(mid, str) and mid: + ids.append(mid) return ids -def _probe_openai_models(base_url: str, - api_key: Any, - *, - token_provider: Optional[Callable[[], str]] = None, - ) -> tuple[bool, list[str]]: +def _probe_openai_models(base_url: str, api_key: Any, *, token_provider: TokenProvider = None) -> tuple[bool, list[str]]: """Probe ``/models`` for an OpenAI-shaped response.""" base_url = base_url.rstrip("/") - - # Azure OpenAI v1: {resource}.openai.azure.com/openai/v1 — no - # api-version required for GA paths, so probe without first. - # Fallback: explicit api-version for pre-v1 resources - candidates = [f"{base_url}/models"] + [ - f"{base_url}/models?api-version={v}" for v in _AZURE_OPENAI_PROBE_API_VERSIONS - ] + # Azure OpenAI v1 needs no api-version for GA paths, so probe without first; then fall back to + # explicit api-versions for pre-v1 resources. + candidates = [f"{base_url}/models"] + [f"{base_url}/models?api-version={v}" for v in _AZURE_OPENAI_PROBE_API_VERSIONS] for url in candidates: status, body = _http_get_json(url, api_key, token_provider=token_provider) if status == 200 and body is not None: ids = _extract_model_ids(body) if ids: - logger.info( - "azure_detect: /models probe OK at %s (%d models)", - url, len(ids), - ) + logger.info("azure_detect: /models probe OK at %s (%d models)", url, len(ids)) return True, ids - # 200 + empty list still counts as "OpenAI shape, no models - # listed" — let the user proceed with manual entry. + # 200 + empty list still counts as "OpenAI shape, no models listed". if isinstance(body, dict) and "data" in body: return True, [] return False, [] -def _probe_anthropic_messages(base_url: str, - api_key: Any, - *, - token_provider: Optional[Callable[[], str]] = None, - ) -> bool: - """Send a zero-token request to ``/v1/messages`` and check whether the endpoint at least - *recognises* the Anthropic Messages shape (any 4xx that mentions ``messages`` or ``model``, or a - 400 ``invalid_request`` with an Anthropic error shape). Never completes a real chat. +def _probe_anthropic_messages(base_url: str, api_key: Any, *, token_provider: TokenProvider = None) -> bool: + """Zero-token POST to ``/v1/messages``: does the endpoint *recognise* the Anthropic shape? + + Any 4xx mentioning ``messages``/``model``, or an Anthropic-shaped error body, counts. Never + completes a real chat. """ - base = _strip_trailing_v1(base_url) - url = f"{base}/v1/messages?api-version={_AZURE_ANTHROPIC_API_VERSION}" - payload = json.dumps({ - "model": "probe", - "max_tokens": 1, - "messages": [{"role": "user", "content": "ping"}], - }).encode("utf-8") + url = f"{_strip_trailing_v1(base_url)}/v1/messages?api-version={_AZURE_ANTHROPIC_API_VERSION}" + payload = json.dumps({"model": "probe", "max_tokens": 1, "messages": [{"role": "user", "content": "ping"}]}).encode("utf-8") req = _authed_request(url, api_key, token_provider, method="POST", data=payload) req.add_header("anthropic-version", "2023-06-01") req.add_header("content-type", "application/json") try: with open_credentialed_url(req, timeout=6.0) as resp: - # Should never 200 — "probe" isn't a real deployment. But - # if it does, the endpoint definitely speaks Anthropic. + # Should never 200 — "probe" isn't a real deployment — but if it does, it speaks Anthropic. return resp.status < 500 except HTTPError as exc: - # 4xx with an Anthropic-shaped error body = Anthropic endpoint. try: - body = exc.read().decode("utf-8", errors="replace") - lowered = body.lower() + lowered = exc.read().decode("utf-8", errors="replace").lower() if "anthropic" in lowered or '"type"' in lowered and '"error"' in lowered: return True - # Pre-Azure-v1 Azure Foundry returns a plain 404 for - # Anthropic-style calls on non-Anthropic deployments. A - # 400 "model not found" IS Anthropic though. - if exc.code == 400 and ("messages" in lowered or "model" in lowered): - return True - return False + # Pre-Azure-v1 Foundry returns a plain 404 for Anthropic-style calls on non-Anthropic + # deployments. A 400 "model not found" IS Anthropic though. + return exc.code == 400 and ("messages" in lowered or "model" in lowered) except Exception: return False - except (URLError, TimeoutError, OSError): - return False - except Exception: # pragma: no cover + except Exception: # URLError, TimeoutError, OSError, anything else return False -def detect(base_url: str, - api_key: Any = "", - *, - token_provider: Optional[Callable[[], str]] = None, - ) -> DetectionResult: - """Inspect an Azure endpoint and describe its transport + models. - - Call this from the wizard before asking the user to pick an API mode manually. The caller should - treat the returned :class:`DetectionResult` as *advisory* — if ``api_mode`` is None, fall back - to asking the user. +def detect(base_url: str, api_key: Any = "", *, token_provider: TokenProvider = None) -> DetectionResult: + """Inspect an Azure endpoint and describe its transport + models (advisory — None api_mode means ask the user). ``api_key`` may be a string (legacy API-key auth — sends both ``api-key:`` and ``Authorization: Bearer``) or a callable returning a bearer JWT (Entra ID auth — sends ONLY ``Authorization: - Bearer``). ``token_provider`` is an alternative explicit name for the callable form; if both are - supplied the callable wins. + Bearer``). ``token_provider`` is an explicit name for the callable form; the callable wins. """ result = DetectionResult() - try: - parsed = urlparse(base_url) - result.hostname = (parsed.hostname or "").lower() + result.hostname = (urlparse(base_url).hostname or "").lower() except Exception: result.hostname = "" - # 1. Path sniff. Azure Foundry exposes Anthropic-style deployments - # under a dedicated ``/anthropic`` path. + # 1. Path sniff: Foundry exposes Anthropic-style deployments under a dedicated /anthropic path. if _looks_like_anthropic_path(base_url): result.is_anthropic = True result.api_mode = "anthropic_messages" result.reason = "URL path ends in /anthropic → Anthropic Messages API" return result - # 2. Try the OpenAI-style /models probe. If this works, the - # endpoint definitely speaks OpenAI wire. + # 2. OpenAI-style /models probe — success means the endpoint definitely speaks OpenAI wire. ok, models = _probe_openai_models(base_url, api_key, token_provider=token_provider) if ok: result.models_probe_ok = True result.models = models result.api_mode = "chat_completions" result.reason = ( - f"GET /models returned {len(models)} model(s) — OpenAI-style endpoint" - if models + f"GET /models returned {len(models)} model(s) — OpenAI-style endpoint" if models else "GET /models returned an OpenAI-shaped empty list — OpenAI-style endpoint" ) return result - # 3. Fallback: probe the Anthropic Messages shape. Slower and more - # intrusive than /models, so only run it when the OpenAI probe - # failed. + # 3. Anthropic Messages probe — slower and more intrusive, so only when /models failed. if _probe_anthropic_messages(base_url, api_key, token_provider=token_provider): result.is_anthropic = True result.api_mode = "anthropic_messages" result.reason = "Endpoint accepts Anthropic Messages shape" return result - # Nothing matched. Caller falls back to manual selection. result.reason = ( "Could not probe endpoint (private network, missing model list, or " "non-standard path) — falling back to manual API-mode selection" @@ -301,40 +229,27 @@ def detect(base_url: str, return result -def lookup_context_length(model: str, - base_url: str, - api_key: Any = "", - *, - token_provider: Optional[Callable[[], str]] = None, - ) -> Optional[int]: - """Thin wrapper around :func:`agent.model_metadata.get_model_context_length` that returns ``None`` - when only the fallback default (128k) would fire, so the wizard can distinguish "we actually - know this" from "we guessed. +def lookup_context_length(model: str, base_url: str, api_key: Any = "", *, + token_provider: TokenProvider = None) -> Optional[int]: + """``get_model_context_length`` that returns None when only the fallback default would fire, so + the wizard can distinguish "we actually know this" from "we guessed". """ model_id = str(model or "").strip() if not model_id: return None try: - from agent.model_metadata import ( - DEFAULT_FALLBACK_CONTEXT, - get_model_context_length, - ) + from agent.model_metadata import DEFAULT_FALLBACK_CONTEXT, get_model_context_length except Exception: return None - # Resolve the credential once. For Entra mode this calls the token - # provider; for legacy api_key this is a no-op string pass-through. + # Resolve the credential once: Entra mode calls the provider; api_key is a string pass-through. token, _mode = _resolve_credential(api_key, token_provider) - try: n = get_model_context_length(model_id, base_url=base_url, api_key=token or "") except Exception as exc: logger.debug("azure_detect: context length lookup failed: %s", exc) return None - - if isinstance(n, int) and n > 0 and n != DEFAULT_FALLBACK_CONTEXT: - return n - return None + return n if isinstance(n, int) and n > 0 and n != DEFAULT_FALLBACK_CONTEXT else None __all__ = ["DetectionResult", "detect", "lookup_context_length"] From 852178c312a342d4c7ad33688416eb2f10df47ab Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 20:36:40 -0700 Subject: [PATCH 07/16] =?UTF-8?q?refactor(hermes=5Fcli):=20blueprint=5Fcmd?= =?UTF-8?q?=20=E2=80=94=20match=20passes=20as=20ordered=20table,=20simpler?= =?UTF-8?q?=20kv=20parse?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/blueprint_cmd.py | 122 +++++++++++++----------------------- 1 file changed, 43 insertions(+), 79 deletions(-) diff --git a/hermes_cli/blueprint_cmd.py b/hermes_cli/blueprint_cmd.py index 187f2a4970..d51cd42fb7 100644 --- a/hermes_cli/blueprint_cmd.py +++ b/hermes_cli/blueprint_cmd.py @@ -49,22 +49,29 @@ def _parse_kv(tokens) -> Tuple[Dict[str, str], list]: values: Dict[str, str] = {} leftovers = [] for tok in tokens: - if "=" in tok: - k, _, v = tok.partition("=") - k = k.strip() - if k: - values[k] = v.strip() - continue - leftovers.append(tok) + k, sep, v = tok.partition("=") + if sep and k.strip(): + values[k.strip()] = v.strip() + else: + leftovers.append(tok) return values, leftovers +def _pick(candidates: List[Any]) -> Optional[Tuple[Optional[Any], List[Any]]]: + """One candidate -> (it, []); several -> (None, all); none -> None (keep searching).""" + if len(candidates) == 1: + return candidates[0], [] + if candidates: + return None, candidates + return None + + def match_blueprint(query: str) -> Tuple[Optional[Any], List[Any]]: """Resolve a free-typed blueprint name to a blueprint. Matching is forgiving because chat-line users type the name (unlike the dashboard/Discord where - it's picked): exact key first, then case-insensitive prefix on key or title, then a difflib - fuzzy pass. + it's picked): exact key first, then case-insensitive prefix on key or title, then substring + anywhere in key/title/description, then a difflib fuzzy pass on keys. """ from cron.blueprint_catalog import CATALOG, get_blueprint @@ -76,35 +83,15 @@ def match_blueprint(query: str) -> Tuple[Optional[Any], List[Any]]: if exact is not None: return exact, [] - # Prefix match on key or title word-start. - prefix = [ - r for r in CATALOG - if r.key.lower().startswith(q) - or any(w.lower().startswith(q) for w in r.title.split()) - ] - if len(prefix) == 1: - return prefix[0], [] - if len(prefix) > 1: - return None, prefix - - # Substring match anywhere in key/title/description. - substr = [ - r for r in CATALOG - if q in r.key.lower() or q in r.title.lower() or q in r.description.lower() - ] - if len(substr) == 1: - return substr[0], [] - if len(substr) > 1: - return None, substr - - # Fuzzy on keys (typo tolerance). - keys = [r.key for r in CATALOG] - close = difflib.get_close_matches(q, keys, n=3, cutoff=0.6) - if len(close) == 1: - return get_blueprint(close[0]), [] - if len(close) > 1: - return None, [get_blueprint(k) for k in close] - + passes = ( + lambda: [r for r in CATALOG if r.key.lower().startswith(q) or any(w.lower().startswith(q) for w in r.title.split())], + lambda: [r for r in CATALOG if q in r.key.lower() or q in r.title.lower() or q in r.description.lower()], + lambda: [get_blueprint(k) for k in difflib.get_close_matches(q, [r.key for r in CATALOG], n=3, cutoff=0.6)], + ) + for candidates in passes: + picked = _pick(candidates()) + if picked is not None: + return picked return None, [] @@ -126,16 +113,13 @@ def build_blueprint_seed(blueprint) -> str: """ from cron.blueprint_catalog import WEEKDAY_PRESETS - lines: List[str] = [] - lines.append( + lines: List[str] = [ f"Set up the '{blueprint.title}' automation for me (automation blueprint " - f"'{blueprint.key}'). {blueprint.description}" - ) - lines.append("") - lines.append( + f"'{blueprint.key}'). {blueprint.description}", + "", "Ask me for each of these, one at a time, offering the default in " - "brackets if I don't have a preference:" - ) + "brackets if I don't have a preference:", + ] for s in blueprint.slots: bits = [f"- {s.label} ({s.name})"] if s.options: @@ -178,8 +162,7 @@ def _fmt_catalog() -> str: def _fmt_candidates(query: str, candidates: List[Any]) -> str: lines = [f"'{query}' matches several blueprints — which one?\n"] - for r in candidates: - lines.append(f" • {r.key} — {r.title}") + lines.extend(f" • {r.key} — {r.title}" for r in candidates) lines.append("\nRun `/blueprint ` with one of the names above.") return "\n".join(lines) @@ -187,22 +170,16 @@ def _fmt_candidates(query: str, candidates: List[Any]) -> str: def _fmt_no_match(query: str) -> str: from cron.blueprint_catalog import CATALOG - keys = [r.key for r in CATALOG] - close = difflib.get_close_matches((query or "").lower(), keys, n=3, cutoff=0.4) + close = difflib.get_close_matches((query or "").lower(), [r.key for r in CATALOG], n=3, cutoff=0.4) msg = f"No automation blueprint matches '{query}'." if close: msg += " Did you mean: " + ", ".join(close) + "?" - msg += " Run /blueprint to see the catalog." - return msg + return msg + " Run /blueprint to see the catalog." def _manage_hint(surface: str) -> str: - """Post-create management hint. /cron is a CLI-only slash command; on gateway platforms the user - manages jobs by asking the agent (cronjob tool) or from the dashboard. - """ - if surface == "cli": - return "Manage it with /cron." - return "Ask me to list, pause, or remove it any time." + """/cron is CLI-only; on gateway platforms jobs are managed via the agent (cronjob tool) or dashboard.""" + return "Manage it with /cron." if surface == "cli" else "Ask me to list, pause, or remove it any time." def handle_blueprint_command( @@ -213,13 +190,10 @@ def handle_blueprint_command( ) -> BlueprintCommandResult: """Dispatch a ``/blueprint`` invocation. - Returns a :class:`BlueprintCommandResult`. When ``agent_seed`` is set the caller must feed it to - the agent as the next user turn; otherwise the command is fully handled and only ``text`` is - shown. - - ``args`` is everything after ``/blueprint``. ``origin`` lets a directly created job deliver back - to the chat it was set up from. ``surface`` (``"cli"`` | ``"gateway"``) picks the right wording - for follow-up hints — ``/cron`` only exists on the CLI. + When ``agent_seed`` is set on the result the caller must feed it to the agent as the next user + turn; otherwise the command is fully handled and only ``text`` is shown. ``args`` is everything + after ``/blueprint``; ``origin`` lets a directly created job deliver back to the chat it was set + up from; ``surface`` (``"cli"`` | ``"gateway"``) picks the follow-up hint wording. """ try: from cron.blueprint_catalog import fill_blueprint, BlueprintFillError @@ -232,7 +206,6 @@ def handle_blueprint_command( except ValueError: tokens = (args or "").split() - # Bare -> list catalog. if not tokens: return BlueprintCommandResult(_fmt_catalog()) @@ -241,18 +214,12 @@ def handle_blueprint_command( blueprint, candidates = match_blueprint(query) if blueprint is None: - if candidates: - return BlueprintCommandResult(_fmt_candidates(query, candidates)) - return BlueprintCommandResult(_fmt_no_match(query)) + return BlueprintCommandResult(_fmt_candidates(query, candidates) if candidates else _fmt_no_match(query)) # `` with no inline slot values -> seed the agent to ask for them. if not values: - seed = build_blueprint_seed(blueprint) - text = ( - f"Setting up '{blueprint.title}' ({_humanize_schedule(blueprint)}). " - "I'll ask you a couple of things…" - ) - return BlueprintCommandResult(text, agent_seed=seed) + text = f"Setting up '{blueprint.title}' ({_humanize_schedule(blueprint)}). I'll ask you a couple of things…" + return BlueprintCommandResult(text, agent_seed=build_blueprint_seed(blueprint)) # ` slot=val …` -> fill + create directly (deterministic shortcut). try: @@ -264,10 +231,7 @@ def handle_blueprint_command( ) try: - from cron.scheduler import ( - CronSchedulerRegistrationError, - create_job_with_scheduler_registration, - ) + from cron.scheduler import CronSchedulerRegistrationError, create_job_with_scheduler_registration job = create_job_with_scheduler_registration(**spec) except CronSchedulerRegistrationError as e: From fe6cefd2d981302d795ac64ca64b66c6f3f87c3f Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 20:45:46 -0700 Subject: [PATCH 08/16] =?UTF-8?q?refactor(hermes=5Fcli):=20approvals=5Fsug?= =?UTF-8?q?gest/test,=20approval=5Fmode/transport=20=E2=80=94=20json=20hel?= =?UTF-8?q?per,=20=5Fdeny=20helper,=20packed=20layout?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/approval_mode.py | 38 +++------ hermes_cli/approval_transport.py | 33 ++++---- hermes_cli/approvals_suggest.py | 130 ++++++++++++------------------- hermes_cli/approvals_test.py | 28 ++----- 4 files changed, 79 insertions(+), 150 deletions(-) diff --git a/hermes_cli/approval_mode.py b/hermes_cli/approval_mode.py index fe5d5c24fe..8e835e0b3d 100644 --- a/hermes_cli/approval_mode.py +++ b/hermes_cli/approval_mode.py @@ -37,24 +37,14 @@ def run_approval_mode_command(requested_mode: Optional[str]) -> ApprovalModeResu requested = (requested_mode or "").strip().lower() if not requested: - return ApprovalModeResult( - True, - current, - False, - f"Approval mode: {current} (persistent profile setting).", - ) + return ApprovalModeResult(True, current, False, f"Approval mode: {current} (persistent profile setting).") if requested not in VALID_APPROVAL_MODES: - return ApprovalModeResult( - False, - current, - False, - "Usage: /approvals [manual|smart|off]", - ) + return ApprovalModeResult(False, current, False, "Usage: /approvals [manual|smart|off]") - # set_config_value is the canonical managed-scope/write-safety chokepoint. - # It reports managed policy through stderr + SystemExit, and the fail-closed - # write guard raises RuntimeError on an unparseable config.yaml; capture both - # for slash-command output instead of terminating the interactive worker. + # set_config_value is the canonical managed-scope/write-safety chokepoint. It reports managed + # policy through stderr + SystemExit, and the fail-closed write guard raises RuntimeError on an + # unparseable config.yaml; capture both for slash-command output instead of terminating the + # interactive worker. from hermes_cli.config import set_config_value output = StringIO() @@ -65,24 +55,14 @@ def run_approval_mode_command(requested_mode: Optional[str]) -> ApprovalModeResu detail = output.getvalue().strip() or "Approval mode is managed and cannot be changed." return ApprovalModeResult(False, current, False, detail) except Exception as exc: - return ApprovalModeResult( - False, - current, - False, - f"Failed to save approval mode: {exc}", - ) + return ApprovalModeResult(False, current, False, f"Failed to save approval mode: {exc}") effective = _effective_mode() if effective != requested: return ApprovalModeResult( - False, - effective, - False, + False, effective, False, f"Approval mode remains {effective}; the requested value did not become effective.", ) return ApprovalModeResult( - True, - effective, - effective != current, - f"Approval mode: {effective} (persistent profile setting).", + True, effective, effective != current, f"Approval mode: {effective} (persistent profile setting).", ) diff --git a/hermes_cli/approval_transport.py b/hermes_cli/approval_transport.py index eafa6ee873..64b7a75520 100644 --- a/hermes_cli/approval_transport.py +++ b/hermes_cli/approval_transport.py @@ -25,9 +25,7 @@ _MAX_ACTIVE_TRANSPORT_WORKERS = 8 _transport_worker_slots = threading.BoundedSemaphore(_MAX_ACTIVE_TRANSPORT_WORKERS) ApprovalChoice = Literal["once", "session", "always", "deny"] -ApprovalPresentFn = Callable[ - ["ApprovalRequest"], "ApprovalDecision | Awaitable[ApprovalDecision]" -] +ApprovalPresentFn = Callable[["ApprovalRequest"], "ApprovalDecision | Awaitable[ApprovalDecision]"] @dataclass(frozen=True) @@ -81,9 +79,7 @@ class ApprovalRequest: ) canonical = json.dumps({**fields, "session_key": session_key}, sort_keys=True, separators=(",", ":")) digest = hashlib.sha256(canonical.encode("utf-8")).hexdigest() - return cls( - **{**fields, "pattern_keys": pattern_keys, "allowed_choices": tuple(choices)}, digest=digest - ) + return cls(**{**fields, "pattern_keys": pattern_keys, "allowed_choices": tuple(choices)}, digest=digest) def respond(self, choice: ApprovalChoice | str) -> ApprovalDecision: """Build the correlated response a transport should return.""" @@ -108,6 +104,10 @@ class RegisteredApprovalTransport: profile_home: str +def _deny(failure: str) -> ApprovalTransportResult: + return ApprovalTransportResult("deny", failure) + + def invoke_approval_transport( present: ApprovalPresentFn, request: ApprovalRequest, @@ -123,10 +123,9 @@ def invoke_approval_transport( loop. A callback must return before the host timeout; late results are discarded and cannot authorize another request. """ - if not _transport_worker_slots.acquire(blocking=False): logger.warning("Approval transport worker capacity exhausted") - return ApprovalTransportResult("deny", "busy") + return _deny("busy") results: queue.Queue[tuple[str, object, float]] = queue.Queue(maxsize=1) deadline = time.monotonic() + max(float(timeout_seconds), 0.0) @@ -148,27 +147,23 @@ def invoke_approval_transport( finally: _transport_worker_slots.release() - worker = threading.Thread( - target=_run, name=f"approval-transport-{request.request_id[:8]}", daemon=True - ) + worker = threading.Thread(target=_run, name=f"approval-transport-{request.request_id[:8]}", daemon=True) try: worker.start() except BaseException: _transport_worker_slots.release() logger.warning("Could not start approval transport worker") - return ApprovalTransportResult("deny", "error") + return _deny("error") while True: if is_interrupted is not None and is_interrupted(): logger.info("Approval transport wait interrupted for %s", request.request_id) - return ApprovalTransportResult("deny", "interrupted") + return _deny("interrupted") remaining = deadline - time.monotonic() if remaining <= 0: logger.warning("Approval transport timed out for request %s", request.request_id) - return ApprovalTransportResult("deny", "timeout") + return _deny("timeout") try: - kind, value, completed_at = results.get( - timeout=min(max(float(poll_interval), 0.001), remaining) - ) + kind, value, completed_at = results.get(timeout=min(max(float(poll_interval), 0.001), remaining)) break except queue.Empty: if on_poll is not None: @@ -178,9 +173,7 @@ def invoke_approval_transport( logger.debug("Approval transport poll callback failed", exc_info=True) failure = _validate_decision(kind, value, completed_at, deadline, request) - if failure is not None: - return ApprovalTransportResult("deny", failure) - return ApprovalTransportResult(value.choice) + return _deny(failure) if failure is not None else ApprovalTransportResult(value.choice) def _validate_decision(kind, value, completed_at, deadline, request) -> str | None: diff --git a/hermes_cli/approvals_suggest.py b/hermes_cli/approvals_suggest.py index af92627eef..e0a432d9ed 100644 --- a/hermes_cli/approvals_suggest.py +++ b/hermes_cli/approvals_suggest.py @@ -2,12 +2,10 @@ Hermes has no dedicated approval-decision ledger: ``always`` answers land in ``command_allowlist`` (config.yaml) via :func:`tools.approval.save_permanent_allowlist`, while ``once``/``session`` -approvals are in-memory only. - -So this module mines *implied approvals*: a command that matches a dangerous-command class (the same -:func:`tools.approval.detect_dangerous_command` classifier that triggers the prompt) AND whose tool -result is not a block/denial marker must have been approved by the user (once, session, always, -smart-approve, or yolo) before it ran. +approvals are in-memory only. So this module mines *implied approvals*: a command that matches a +dangerous-command class (the same :func:`tools.approval.detect_dangerous_command` classifier that +triggers the prompt) AND whose tool result is not a block/denial marker must have been approved by +the user (once, session, always, smart-approve, or yolo) before it ran. """ from __future__ import annotations @@ -25,12 +23,10 @@ from typing import Iterable, Iterator, Optional # Safety exclusions # --------------------------------------------------------------------------- -# Dangerous-class descriptions matching ANY of these are never proposed, -# regardless of approval frequency. Matched case-insensitively against the -# pattern-key/description strings produced by tools.approval's -# DANGEROUS_PATTERNS / execution-flag findings. Deliberately conservative: -# a benign class accidentally excluded costs the user one manual config edit; -# a destructive class accidentally proposed costs them data. +# Dangerous-class descriptions matching ANY of these are never proposed, regardless of approval +# frequency (matched case-insensitively against tools.approval's DANGEROUS_PATTERNS / execution-flag +# descriptions). Deliberately conservative: a benign class accidentally excluded costs the user +# one manual config edit; a destructive class accidentally proposed costs them data. _UNSAFE_CLASS_PATTERNS = [ r"delete", # recursive delete, find -delete, branch force delete, ... r"\brm\b", # xargs with rm, find -exec rm @@ -71,8 +67,8 @@ _UNSAFE_CLASS_PATTERNS = [ ] _UNSAFE_CLASS_RE = re.compile("|".join(_UNSAFE_CLASS_PATTERNS), re.IGNORECASE) -# Root binaries that must never anchor a proposed command glob, even if the -# class survived the description filter. Prefix match for the mkfs family. +# Root binaries that must never anchor a proposed command glob, even if the class survived the +# description filter. Prefix match for the mkfs family. _UNSAFE_ROOT_BINARIES = { "rm", "rmdir", "unlink", "shred", "dd", "fdisk", "parted", "wipefs", "sudo", "doas", "su", "chmod", "chown", "chgrp", @@ -82,9 +78,8 @@ _UNSAFE_ROOT_BINARIES = { } _UNSAFE_ROOT_PREFIXES = ("mkfs",) -# Substrings in a role='tool' result that mean the command did NOT execute -# with user consent (blocked, denied, timed out, or still pending). Kept in -# sync with the message templates in tools/approval.py. +# Substrings in a role='tool' result that mean the command did NOT execute with user consent +# (blocked, denied, timed out, or still pending). Kept in sync with tools/approval.py templates. _BLOCK_MARKERS = ( "BLOCKED (hardline)", "BLOCKED: User denied", @@ -128,22 +123,23 @@ def default_db_path() -> Path: def _connect_readonly(db_path: Path) -> sqlite3.Connection: - uri = f"file:{db_path}?mode=ro" - return sqlite3.connect(uri, uri=True) + return sqlite3.connect(f"file:{db_path}?mode=ro", uri=True) def _fetch_rows(cur) -> Iterator[tuple]: """Stream cursor rows in 2000-row batches.""" - while True: - rows = cur.fetchmany(2000) - if not rows: - return + while rows := cur.fetchmany(2000): yield from rows -def _iter_terminal_calls( - con: sqlite3.Connection, since_ts: float -) -> Iterator[tuple[str, str]]: +def _json_or_none(raw): + try: + return json.loads(raw) + except (TypeError, ValueError): + return None + + +def _iter_terminal_calls(con: sqlite3.Connection, since_ts: float) -> Iterator[tuple[str, str]]: """Yield ``(tool_call_id, command)`` for every terminal tool call.""" cur = con.execute( "SELECT tool_calls FROM messages " @@ -152,21 +148,13 @@ def _iter_terminal_calls( (since_ts,), ) for (raw,) in _fetch_rows(cur): - try: - calls = json.loads(raw) - except (TypeError, ValueError): - continue - if not isinstance(calls, list): - continue - for call in calls: - if not isinstance(call, dict): - continue - fn = call.get("function") or {} + calls = _json_or_none(raw) + for call in calls if isinstance(calls, list) else (): + fn = call.get("function") or {} if isinstance(call, dict) else {} if fn.get("name") != "terminal": continue - try: - args = json.loads(fn.get("arguments") or "{}") - except (TypeError, ValueError): + args = _json_or_none(fn.get("arguments") or "{}") + if args is None: continue command = args.get("command") if isinstance(command, str) and command.strip(): @@ -188,12 +176,9 @@ def _blocked_tool_call_ids(con: sqlite3.Connection, since_ts: float) -> set: } -def scan_approval_history( - db_path: Optional[Path] = None, days: int = 90 -) -> list[tuple[str, str]]: - """Return ``(command, dangerous_class_description)`` records mined from the session DB — - dangerous-classified terminal commands that actually executed (i.e. carried an implied user - approval). +def scan_approval_history(db_path: Optional[Path] = None, days: int = 90) -> list[tuple[str, str]]: + """``(command, dangerous_class_description)`` records for dangerous-classified terminal commands + that actually executed (i.e. carried an implied user approval). """ from tools.approval import detect_dangerous_command, detect_hardline_command @@ -210,15 +195,13 @@ def scan_approval_history( for tool_call_id, command in _iter_terminal_calls(con, since_ts): if tool_call_id in blocked: continue - is_hardline, _desc = detect_hardline_command(command) - if is_hardline: - # Hardline commands are unconditionally blocked at runtime; - # never mine them (defense in depth against stale DB rows). + # Hardline commands are unconditionally blocked at runtime; never mine them (defense in + # depth against stale DB rows). + if detect_hardline_command(command)[0]: continue is_dangerous, _key, description = detect_dangerous_command(command) - if not is_dangerous: - continue - records.append((command, description)) + if is_dangerous: + records.append((command, description)) finally: con.close() return records @@ -230,13 +213,9 @@ def scan_approval_history( def normalize_command(command: str) -> str: """Fold user/hermes home prefixes and collapse whitespace.""" - from tools.approval import ( - _rewrite_resolved_hermes_home, - _rewrite_resolved_user_home, - ) + from tools.approval import _rewrite_resolved_hermes_home, _rewrite_resolved_user_home - folded = _rewrite_resolved_user_home(_rewrite_resolved_hermes_home(command)) - return " ".join(folded.split()) + return " ".join(_rewrite_resolved_user_home(_rewrite_resolved_hermes_home(command)).split()) def is_unsafe_class(description: str) -> bool: @@ -257,12 +236,8 @@ def derive_glob(normalized: str) -> Optional[str]: """ from tools.approval import _has_allowlist_shell_operator - if _has_allowlist_shell_operator(normalized): - return None tokens = normalized.split() - if not tokens: - return None - if _unsafe_root_binary(tokens[0]): + if _has_allowlist_shell_operator(normalized) or not tokens or _unsafe_root_binary(tokens[0]): return None if len(tokens) == 1: return tokens[0] @@ -292,19 +267,15 @@ def build_proposals( continue normalized = normalize_command(command) glob = derive_glob(normalized) - key = (glob, "glob") if glob is not None else (description, "class") - pattern, kind = key + pattern, kind = (glob, "glob") if glob is not None else (description, "class") if pattern in existing: continue - proposal = by_pattern.get(key) - if proposal is None: - proposal = by_pattern[key] = Proposal(pattern=pattern, kind=kind) + proposal = by_pattern.setdefault((pattern, kind), Proposal(pattern=pattern, kind=kind)) proposal.count += 1 proposal.classes.add(description) proposal.add_example(normalized) - ranked = [p for p in by_pattern.values() if p.count >= max(min_count, 1)] - ranked.sort(key=lambda p: (-p.count, p.pattern)) + ranked = sorted((p for p in by_pattern.values() if p.count >= max(min_count, 1)), key=lambda p: (-p.count, p.pattern)) return ranked[: max(limit, 1)] @@ -336,12 +307,10 @@ def apply_proposals(proposals: list[Proposal], indices: list[int]) -> set: """Merge chosen proposal patterns into command_allowlist and persist.""" import tools.approval as approval_module - merged = set(approval_module.load_permanent_allowlist()) - for idx in indices: - merged.add(proposals[idx].pattern) + merged = set(approval_module.load_permanent_allowlist()) | {proposals[idx].pattern for idx in indices} approval_module.save_permanent_allowlist(merged) - # Keep the in-process allowlist consistent so a long-lived process sees - # the new entries immediately (mirrors the interactive 'always' path). + # Keep the in-process allowlist consistent so a long-lived process sees the new entries + # immediately (mirrors the interactive 'always' path). approval_module.load_permanent(merged) return merged @@ -382,13 +351,13 @@ def suggest_command(args) -> int: import tools.approval as approval_module existing = set(approval_module.load_permanent_allowlist()) - records = scan_approval_history(db_path, days=days) proposals = build_proposals( - records, + scan_approval_history(db_path, days=days), existing=existing, min_count=getattr(args, "min_count", 2), limit=getattr(args, "limit", 20), ) + as_json = getattr(args, "json", False) apply_spec = getattr(args, "apply_indices", None) if apply_spec: @@ -399,17 +368,16 @@ def suggest_command(args) -> int: return 1 merged = apply_proposals(proposals, indices) applied = [proposals[i].pattern for i in indices] - if getattr(args, "json", False): + if as_json: print(json.dumps({"applied": applied, "allowlist_size": len(merged)})) else: print("Added to command_allowlist:") for pattern in applied: print(f" + {pattern}") - print(f"\ncommand_allowlist now has {len(merged)} entries " - "(~/.hermes/config.yaml).") + print(f"\ncommand_allowlist now has {len(merged)} entries (~/.hermes/config.yaml).") return 0 - if getattr(args, "json", False): + if as_json: payload = { "db": str(db_path), "days": days, diff --git a/hermes_cli/approvals_test.py b/hermes_cli/approvals_test.py index c9bca6d57a..6400c61cab 100644 --- a/hermes_cli/approvals_test.py +++ b/hermes_cli/approvals_test.py @@ -7,7 +7,7 @@ order the runtime guard (``check_all_command_guards``) applies them: 1. container-skip gate (isolated backends bypass all guards), 2. hardline blocklist (never bypassable, fires before yolo/off), 3. sudo-stdin guard (unconditional), 4. user ``approvals.deny`` rules (fire before yolo/off), 5. yolo / ``approvals.mode: off`` bypass, 6. permanent -``command_allowlist``, 7. +``command_allowlist``, 7. dangerous-pattern detection (would prompt). """ from __future__ import annotations @@ -36,8 +36,8 @@ def evaluate_command(command: str, env_type: str = "local") -> dict: """ import tools.approval as approval - # Sync config-persisted "always" patterns so the allowlist check below - # sees what the runtime would see (load is read-only). + # Sync config-persisted "always" patterns so the allowlist check below sees what the runtime + # would see (load is read-only). try: approval.load_permanent_allowlist() except Exception: @@ -56,8 +56,7 @@ def evaluate_command(command: str, env_type: str = "local") -> dict: "normalized_variants": variants, } - # 1. Isolated container backends skip every guard (runtime parity: - # this fires BEFORE the hardline floor in check_all_command_guards). + # 1. Isolated container backends skip every guard (fires BEFORE the hardline floor at runtime). if approval._should_skip_container_guards(env_type): return result( "allow", @@ -77,10 +76,7 @@ def evaluate_command(command: str, env_type: str = "local") -> dict: # 3. Sudo stdin guard — unconditional, like the hardline floor. is_sudo_guess, sudo_desc = approval._check_sudo_stdin_guard(command) if is_sudo_guess: - return result( - "hardline-deny", rule=sudo_desc, - detail="sudo stdin guard (unconditional block)", - ) + return result("hardline-deny", rule=sudo_desc, detail="sudo stdin guard (unconditional block)") # 4. User-defined approvals.deny rules — fire before yolo/off. deny_pattern = approval._match_user_deny_rule(command) @@ -103,11 +99,7 @@ def evaluate_command(command: str, env_type: str = "local") -> dict: # 6. Permanent command_allowlist. if approval._command_matches_permanent_allowlist(command): - return result( - "allow", - detail="matches command_allowlist in config.yaml " - "(permanently approved)", - ) + return result("allow", detail="matches command_allowlist in config.yaml (permanently approved)") # 7. Dangerous-pattern detection → would prompt. is_dangerous, pattern_key, description = approval.detect_dangerous_command(command) @@ -138,18 +130,14 @@ def _render_text(verdict: dict) -> None: def approvals_test_command(args) -> int: """Handle ``hermes approvals test ``. Returns the exit code.""" words = list(getattr(args, "command_words", None) or []) - # argparse REMAINDER keeps a leading "--" separator; it is not part of - # the command being evaluated. + # argparse REMAINDER keeps a leading "--" separator; it is not part of the command. if words and words[0] == "--": words = words[1:] if not words: print("usage: hermes approvals test [--env-type TYPE] [--json] -- ") return EXIT_USAGE - command = " ".join(words) - env_type = getattr(args, "env_type", None) or "local" - - verdict = evaluate_command(command, env_type=env_type) + verdict = evaluate_command(" ".join(words), env_type=getattr(args, "env_type", None) or "local") if getattr(args, "json", False): print(json.dumps(verdict, indent=2)) else: From 0e3203618b28e3539e413743b16800ade6596207 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 20:56:23 -0700 Subject: [PATCH 09/16] =?UTF-8?q?refactor(hermes=5Fcli):=20group=20C=20?= =?UTF-8?q?=E2=80=94=20AST-neutral=20layout=20compaction=20(hug/pack/join,?= =?UTF-8?q?=20verified=20ast.dump-equal)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/approval_transport.py | 20 ++----- hermes_cli/approvals_suggest.py | 10 ++-- hermes_cli/auth_minimax.py | 3 +- hermes_cli/auth_model_picker.py | 24 +++------ hermes_cli/auth_qwen.py | 7 +-- hermes_cli/auth_spotify.py | 91 ++++++++------------------------ hermes_cli/auth_xai.py | 53 +++++-------------- hermes_cli/blueprint_cmd.py | 5 +- 8 files changed, 52 insertions(+), 161 deletions(-) diff --git a/hermes_cli/approval_transport.py b/hermes_cli/approval_transport.py index 64b7a75520..bf141d0b9f 100644 --- a/hermes_cli/approval_transport.py +++ b/hermes_cli/approval_transport.py @@ -54,16 +54,8 @@ class ApprovalRequest: @classmethod def create( - cls, - *, - command: str, - description: str, - pattern_key: str, - pattern_keys: tuple[str, ...], - session_key: str, - surface: str, - allow_session: bool, - allow_permanent: bool, + cls, *, command: str, description: str, pattern_key: str, pattern_keys: tuple[str, ...], + session_key: str, surface: str, allow_session: bool, allow_permanent: bool, timeout_seconds: float = 300, ) -> "ApprovalRequest": choices: list[ApprovalChoice] = ["once"] @@ -109,12 +101,8 @@ def _deny(failure: str) -> ApprovalTransportResult: def invoke_approval_transport( - present: ApprovalPresentFn, - request: ApprovalRequest, - *, - timeout_seconds: float, - poll_interval: float = 1.0, - on_poll: Callable[[], None] | None = None, + present: ApprovalPresentFn, request: ApprovalRequest, *, timeout_seconds: float, + poll_interval: float = 1.0, on_poll: Callable[[], None] | None = None, is_interrupted: Callable[[], bool] | None = None, ) -> ApprovalTransportResult: """Run a sync or async transport on a bounded daemon worker. diff --git a/hermes_cli/approvals_suggest.py b/hermes_cli/approvals_suggest.py index e0a432d9ed..231338cf14 100644 --- a/hermes_cli/approvals_suggest.py +++ b/hermes_cli/approvals_suggest.py @@ -248,9 +248,7 @@ def derive_glob(normalized: str) -> Optional[str]: def build_proposals( - records: Iterable[tuple[str, str]], - existing: Optional[set] = None, - min_count: int = 2, + records: Iterable[tuple[str, str]], existing: Optional[set] = None, min_count: int = 2, limit: int = 20, ) -> list[Proposal]: """Aggregate scan records into a ranked, safety-filtered proposal list. @@ -352,10 +350,8 @@ def suggest_command(args) -> int: existing = set(approval_module.load_permanent_allowlist()) proposals = build_proposals( - scan_approval_history(db_path, days=days), - existing=existing, - min_count=getattr(args, "min_count", 2), - limit=getattr(args, "limit", 20), + scan_approval_history(db_path, days=days), existing=existing, + min_count=getattr(args, "min_count", 2), limit=getattr(args, "limit", 20), ) as_json = getattr(args, "json", False) diff --git a/hermes_cli/auth_minimax.py b/hermes_cli/auth_minimax.py index d7b9b349d4..3dc798ea14 100644 --- a/hermes_cli/auth_minimax.py +++ b/hermes_cli/auth_minimax.py @@ -83,8 +83,7 @@ def _minimax_pkce_pair() -> tuple: def _minimax_request_user_code( - client: httpx.Client, *, portal_base_url: str, client_id: str, - code_challenge: str, state: str, + client: httpx.Client, *, portal_base_url: str, client_id: str, code_challenge: str, state: str ) -> Dict[str, Any]: response = _minimax_post_form( client, diff --git a/hermes_cli/auth_model_picker.py b/hermes_cli/auth_model_picker.py index 2a9497b046..36616ffea9 100644 --- a/hermes_cli/auth_model_picker.py +++ b/hermes_cli/auth_model_picker.py @@ -22,11 +22,7 @@ _CURRENT_SUFFIX = " ← currently in use" def _confirm_selection_guards( - model_id: str, - *, - provider: str = "", - base_url: str = "", - api_key: str = "", + model_id: str, *, provider: str = "", base_url: str = "", api_key: str = "", include_kinds: Optional[List[str]] = None, ) -> bool: """Prompt before saving a model that trips any selection guard (cost, data-policy, ...). @@ -64,12 +60,8 @@ class _ModelPickerRows: """ def __init__( - self, - all_models: List[str], - pricing: Optional[Dict[str, Dict[str, str]]], - *, - current_model: str, - sale_chrome: bool, + self, all_models: List[str], pricing: Optional[Dict[str, Dict[str, str]]], *, + current_model: str, sale_chrome: bool, ) -> None: from hermes_cli.models import _format_price_per_mtok, compute_sale_discount @@ -161,14 +153,10 @@ class _ModelPickerRows: def _prompt_model_selection( - model_ids: List[str], - current_model: str = "", + model_ids: List[str], current_model: str = "", pricing: Optional[Dict[str, Dict[str, str]]] = None, - unavailable_models: Optional[List[str]] = None, - portal_url: str = "", - unavailable_message: str = "", - confirm_provider: str = "", - confirm_base_url: str = "", + unavailable_models: Optional[List[str]] = None, portal_url: str = "", + unavailable_message: str = "", confirm_provider: str = "", confirm_base_url: str = "", confirm_api_key: str = "", ) -> Optional[str]: """Interactive model picker; current_model listed first. Returns the chosen model ID or None. diff --git a/hermes_cli/auth_qwen.py b/hermes_cli/auth_qwen.py index 1f82e3daab..ad15b86300 100644 --- a/hermes_cli/auth_qwen.py +++ b/hermes_cli/auth_qwen.py @@ -67,8 +67,7 @@ def _refresh_qwen_cli_tokens(tokens: Dict[str, Any], timeout_seconds: float = 20 try: response = httpx.post( - QWEN_OAUTH_TOKEN_URL, - headers=_FORM_JSON_HEADERS, + QWEN_OAUTH_TOKEN_URL, headers=_FORM_JSON_HEADERS, data={"grant_type": "refresh_token", "refresh_token": refresh_token, "client_id": QWEN_OAUTH_CLIENT_ID}, timeout=timeout_seconds, ) @@ -121,9 +120,7 @@ def _mark_qwen_oauth_active(creds: Dict[str, Any]) -> None: def resolve_qwen_runtime_credentials( - *, - force_refresh: bool = False, - refresh_if_expiring: bool = True, + *, force_refresh: bool = False, refresh_if_expiring: bool = True, refresh_skew_seconds: int = QWEN_ACCESS_TOKEN_REFRESH_SKEW_SECONDS, ) -> Dict[str, Any]: from hermes_cli.auth import _qwen_cli_auth_path, _refresh_qwen_cli_tokens diff --git a/hermes_cli/auth_spotify.py b/hermes_cli/auth_spotify.py index f8fd068e93..bbf56e4672 100644 --- a/hermes_cli/auth_spotify.py +++ b/hermes_cli/auth_spotify.py @@ -45,22 +45,15 @@ def _spotify_scope_string(raw_scope: Optional[str] = None) -> str: def _spotify_setting( - state: Optional[Dict[str, Any]], - state_key: str, - env_vars: Tuple[str, ...], - default: str, - *, - explicit: Optional[str] = None, - strip_slash: bool = False, + state: Optional[Dict[str, Any]], state_key: str, env_vars: Tuple[str, ...], default: str, *, + explicit: Optional[str] = None, strip_slash: bool = False, ) -> str: """First non-empty of explicit arg, env vars (``.env`` aware), stored state, then *default*.""" from hermes_cli.config import get_env_value candidates = ( - explicit, - *(get_env_value(var) for var in env_vars), - state.get(state_key) if isinstance(state, dict) else None, - default, + explicit, *(get_env_value(var) for var in env_vars), + state.get(state_key) if isinstance(state, dict) else None, default, ) for candidate in candidates: cleaned = _clean(candidate) @@ -113,12 +106,7 @@ def _spotify_code_challenge(code_verifier: str) -> str: def _spotify_build_authorize_url( - *, - client_id: str, - redirect_uri: str, - scope: str, - state: str, - code_challenge: str, + *, client_id: str, redirect_uri: str, scope: str, state: str, code_challenge: str, accounts_base_url: str, ) -> str: query = urlencode({ @@ -204,14 +192,8 @@ def _spotify_wait_for_callback(redirect_uri: str, *, timeout_seconds: float = 18 def _spotify_token_payload_to_state( - token_payload: Dict[str, Any], - *, - client_id: str, - redirect_uri: str, - requested_scope: str, - accounts_base_url: str, - api_base_url: str, - previous_state: Optional[Dict[str, Any]] = None, + token_payload: Dict[str, Any], *, client_id: str, redirect_uri: str, requested_scope: str, + accounts_base_url: str, api_base_url: str, previous_state: Optional[Dict[str, Any]] = None, ) -> Dict[str, Any]: from hermes_cli.auth import _coerce_ttl_seconds now = datetime.now(timezone.utc) @@ -237,15 +219,8 @@ def _spotify_token_payload_to_state( def _spotify_token_post( - accounts_base_url: str, - data: Dict[str, str], - *, - timeout_seconds: float, - what: str, - failed_code: str, - invalid_code: str, - invalid_message: str, - failed_suffix: str = "", + accounts_base_url: str, data: Dict[str, str], *, timeout_seconds: float, what: str, + failed_code: str, invalid_code: str, invalid_message: str, failed_suffix: str = "", relogin_required: bool = False, ) -> Dict[str, Any]: """POST to Spotify's ``/api/token`` and return the JSON payload, or raise a shaped AuthError.""" @@ -272,12 +247,7 @@ def _spotify_token_post( def _spotify_exchange_code_for_tokens( - *, - client_id: str, - code: str, - redirect_uri: str, - code_verifier: str, - accounts_base_url: str, + *, client_id: str, code: str, redirect_uri: str, code_verifier: str, accounts_base_url: str, timeout_seconds: float = 20.0, ) -> Dict[str, Any]: return _spotify_token_post( @@ -310,30 +280,22 @@ def _refresh_spotify_oauth_state(state: Dict[str, Any], *, timeout_seconds: floa payload = _spotify_token_post( accounts_base_url, {"grant_type": "refresh_token", "refresh_token": refresh_token, "client_id": client_id}, - timeout_seconds=timeout_seconds, - what="token refresh", - failed_code="spotify_refresh_failed", + timeout_seconds=timeout_seconds, what="token refresh", failed_code="spotify_refresh_failed", invalid_code="spotify_refresh_invalid", invalid_message="Spotify refresh response did not include an access_token.", - failed_suffix=" Run `hermes auth spotify` again.", - relogin_required=True, + failed_suffix=" Run `hermes auth spotify` again.", relogin_required=True, ) return _spotify_token_payload_to_state( - payload, - client_id=client_id, - redirect_uri=_spotify_redirect_uri(state=state), + payload, client_id=client_id, redirect_uri=_spotify_redirect_uri(state=state), requested_scope=str(state.get("scope") or DEFAULT_SPOTIFY_SCOPE), - accounts_base_url=accounts_base_url, - api_base_url=_spotify_api_base_url(state), + accounts_base_url=accounts_base_url, api_base_url=_spotify_api_base_url(state), previous_state=state, ) def resolve_spotify_runtime_credentials( - *, - force_refresh: bool = False, - refresh_if_expiring: bool = True, + *, force_refresh: bool = False, refresh_if_expiring: bool = True, refresh_skew_seconds: int = SPOTIFY_ACCESS_TOKEN_REFRESH_SKEW_SECONDS, ) -> Dict[str, Any]: from hermes_cli.auth import _auth_store_lock, _is_expiring, _load_auth_store, _load_provider_state, _quarantine_flat_oauth_state, _refresh_spotify_oauth_state, _save_auth_store, _store_provider_state @@ -479,12 +441,8 @@ def login_spotify_command(args) -> None: code_verifier = _spotify_code_verifier() state_nonce = uuid.uuid4().hex authorize_url = _spotify_build_authorize_url( - client_id=client_id, - redirect_uri=redirect_uri, - scope=scope, - state=state_nonce, - code_challenge=_spotify_code_challenge(code_verifier), - accounts_base_url=accounts_base_url, + client_id=client_id, redirect_uri=redirect_uri, scope=scope, state=state_nonce, + code_challenge=_spotify_code_challenge(code_verifier), accounts_base_url=accounts_base_url, ) print( @@ -512,20 +470,13 @@ def login_spotify_command(args) -> None: raise SystemExit("Spotify authorization failed: state mismatch.") token_payload = _spotify_exchange_code_for_tokens( - client_id=client_id, - code=str(callback.get("code") or ""), - redirect_uri=redirect_uri, - code_verifier=code_verifier, - accounts_base_url=accounts_base_url, + client_id=client_id, code=str(callback.get("code") or ""), redirect_uri=redirect_uri, + code_verifier=code_verifier, accounts_base_url=accounts_base_url, timeout_seconds=float(getattr(args, "timeout", None) or 20.0), ) spotify_state = _spotify_token_payload_to_state( - token_payload, - client_id=client_id, - redirect_uri=redirect_uri, - requested_scope=scope, - accounts_base_url=accounts_base_url, - api_base_url=api_base_url, + token_payload, client_id=client_id, redirect_uri=redirect_uri, requested_scope=scope, + accounts_base_url=accounts_base_url, api_base_url=api_base_url, ) with _auth_store_lock(): diff --git a/hermes_cli/auth_xai.py b/hermes_cli/auth_xai.py index 565aeb1e8e..92b546cea2 100644 --- a/hermes_cli/auth_xai.py +++ b/hermes_cli/auth_xai.py @@ -134,12 +134,8 @@ def _write_through_xai_oauth_to_global_root(state: Dict[str, Any]) -> None: def _save_xai_oauth_tokens( - tokens: Dict[str, Any], - *, - discovery: Optional[Dict[str, Any]] = None, - redirect_uri: str = "", - last_refresh: Optional[str] = None, - auth_mode: str = "oauth_device_code", + tokens: Dict[str, Any], *, discovery: Optional[Dict[str, Any]] = None, redirect_uri: str = "", + last_refresh: Optional[str] = None, auth_mode: str = "oauth_device_code", set_active: bool = True, ) -> None: """Persist xAI OAuth tokens into the auth store. @@ -310,10 +306,7 @@ def _xai_tokens_from_payload(payload: Dict[str, Any], access_token: str, fallbac def refresh_xai_oauth_pure( - access_token: str, - refresh_token: str, - *, - token_endpoint: str = "", + access_token: str, refresh_token: str, *, token_endpoint: str = "", timeout_seconds: float = 20.0, ) -> Dict[str, Any]: from hermes_cli.auth import _nonempty_str, _utc_now_z, _xai_oauth_discovery @@ -330,8 +323,7 @@ def refresh_xai_oauth_pure( timeout = httpx.Timeout(max(5.0, float(timeout_seconds))) with httpx.Client(timeout=timeout, headers={"Accept": "application/json"}) as client: response = client.post( - endpoint, - headers={"Content-Type": "application/x-www-form-urlencoded"}, + endpoint, headers={"Content-Type": "application/x-www-form-urlencoded"}, data={"grant_type": "refresh_token", "client_id": XAI_OAUTH_CLIENT_ID, "refresh_token": refresh_token}, ) if response.status_code != 200: @@ -356,11 +348,9 @@ def refresh_xai_oauth_pure( "xAI token refresh failed." + suffix, "xai_refresh_failed", relogin=response.status_code in {400, 401}, ) payload, refreshed_access = _refresh_payload_access_token( - response, - provider="xai-oauth", + response, provider="xai-oauth", invalid_json=("xAI token refresh returned invalid JSON: {exc}", "xai_refresh_invalid_json"), - invalid_json_relogin=False, - strict_str=False, + invalid_json_relogin=False, strict_str=False, invalid_response=("xAI token refresh response was not a JSON object.", "xai_refresh_invalid_response"), missing_access=("xAI token refresh response was missing access_token.", "xai_refresh_missing_access_token"), ) @@ -368,11 +358,7 @@ def refresh_xai_oauth_pure( def _refresh_xai_oauth_tokens( - tokens: Dict[str, Any], - *, - token_endpoint: str, - redirect_uri: str = "", - timeout_seconds: float, + tokens: Dict[str, Any], *, token_endpoint: str, redirect_uri: str = "", timeout_seconds: float ) -> Dict[str, Any]: # Re-persist whatever auth_mode is already stored (legacy pre-device-code logins may still # carry ``oauth_pkce``): the refresh hot path must not relabel how the grant was obtained. @@ -436,9 +422,7 @@ def _xai_oauth_inference_base_url() -> str: def resolve_xai_oauth_runtime_credentials( - *, - force_refresh: bool = False, - refresh_if_expiring: bool = True, + *, force_refresh: bool = False, refresh_if_expiring: bool = True, refresh_skew_seconds: Optional[int] = None, ) -> Dict[str, Any]: from hermes_cli.auth import _auth_store_lock, _is_terminal_xai_oauth_refresh_error, _refresh_xai_oauth_tokens, _xai_oauth_discovery @@ -515,10 +499,8 @@ def _login_xai_oauth(args, pconfig: ProviderConfig, *, force_new_login: bool = F creds = _xai_oauth_device_code_login(timeout_seconds=timeout_seconds, open_browser=open_browser) _save_xai_oauth_tokens( - creds["tokens"], - discovery=creds.get("discovery"), - redirect_uri=creds.get("redirect_uri", ""), - last_refresh=creds.get("last_refresh"), + creds["tokens"], discovery=creds.get("discovery"), + redirect_uri=creds.get("redirect_uri", ""), last_refresh=creds.get("last_refresh"), auth_mode="oauth_device_code", ) # An explicit interactive re-login means the user wants the xAI credential re-enabled. @@ -550,11 +532,7 @@ def _xai_oauth_request_device_code(client: httpx.Client, *, scope: str = XAI_OAU def _xai_oauth_poll_device_token( - client: httpx.Client, - *, - token_endpoint: str, - device_code: str, - expires_in: int, + client: httpx.Client, *, token_endpoint: str, device_code: str, expires_in: int, poll_interval: int, ) -> Dict[str, Any]: from hermes_cli.auth import _poll_device_token_generic @@ -573,8 +551,7 @@ def _xai_oauth_poll_device_token( return _poll_device_token_generic( lambda: client.post( - token_endpoint, - headers=_FORM_JSON_HEADERS, + token_endpoint, headers=_FORM_JSON_HEADERS, data={"grant_type": DEVICE_CODE_GRANT_TYPE, "client_id": XAI_OAUTH_CLIENT_ID, "device_code": device_code}, ), expires_in=int(expires_in), @@ -603,10 +580,8 @@ def _xai_oauth_device_code_login(*, timeout_seconds: float = 20.0, open_browser: ) print(f"Waiting for approval (polling every {max(1, interval)}s)...") payload = _xai_oauth_poll_device_token( - client, - token_endpoint=discovery["token_endpoint"], - device_code=str(device_data["device_code"]), - expires_in=int(device_data["expires_in"]), + client, token_endpoint=discovery["token_endpoint"], + device_code=str(device_data["device_code"]), expires_in=int(device_data["expires_in"]), poll_interval=interval, ) diff --git a/hermes_cli/blueprint_cmd.py b/hermes_cli/blueprint_cmd.py index d51cd42fb7..68a2413045 100644 --- a/hermes_cli/blueprint_cmd.py +++ b/hermes_cli/blueprint_cmd.py @@ -183,10 +183,7 @@ def _manage_hint(surface: str) -> str: def handle_blueprint_command( - args: str, - *, - origin: Optional[Dict[str, Any]] = None, - surface: str = "cli", + args: str, *, origin: Optional[Dict[str, Any]] = None, surface: str = "cli" ) -> BlueprintCommandResult: """Dispatch a ``/blueprint`` invocation. From 1c9275ee83cefc15987019f7f2ac86a384d312e5 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 21:07:20 -0700 Subject: [PATCH 10/16] =?UTF-8?q?refactor(hermes=5Fcli):=20auth=5Fxai=20?= =?UTF-8?q?=E2=80=94=20one=20URL=20classifier=20behind=20both=20origin=20v?= =?UTF-8?q?alidators;=20pack=20marker=20table?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/approvals_suggest.py | 14 +++------ hermes_cli/auth_minimax.py | 1 - hermes_cli/auth_xai.py | 52 ++++++++++++++++++++------------- hermes_cli/auth_zai_kimi.py | 1 - 4 files changed, 35 insertions(+), 33 deletions(-) diff --git a/hermes_cli/approvals_suggest.py b/hermes_cli/approvals_suggest.py index 231338cf14..0dcd67f764 100644 --- a/hermes_cli/approvals_suggest.py +++ b/hermes_cli/approvals_suggest.py @@ -81,16 +81,10 @@ _UNSAFE_ROOT_PREFIXES = ("mkfs",) # Substrings in a role='tool' result that mean the command did NOT execute with user consent # (blocked, denied, timed out, or still pending). Kept in sync with tools/approval.py templates. _BLOCK_MARKERS = ( - "BLOCKED (hardline)", - "BLOCKED: User denied", - "BLOCKED: Action ", - "BLOCKED: Command flagged as dangerous", - "BLOCKED: approval required", - "BLOCKED: Failed to send approval request", - "The user has NOT consented", - "Asking the user for approval", - "approval_required", - "BLOCKED by user deny rule", + "BLOCKED (hardline)", "BLOCKED: User denied", "BLOCKED: Action ", + "BLOCKED: Command flagged as dangerous", "BLOCKED: approval required", + "BLOCKED: Failed to send approval request", "The user has NOT consented", + "Asking the user for approval", "approval_required", "BLOCKED by user deny rule", ) diff --git a/hermes_cli/auth_minimax.py b/hermes_cli/auth_minimax.py index 3dc798ea14..95b51e3fb5 100644 --- a/hermes_cli/auth_minimax.py +++ b/hermes_cli/auth_minimax.py @@ -27,7 +27,6 @@ if TYPE_CHECKING: # annotation-only; the runtime import would be a cycle # Log-record parity with the origin module (caplog tests pin "hermes_cli.auth"). logger = logging.getLogger("hermes_cli.auth") - _MINIMAX_OAUTH_ERROR_BODY_LIMIT = 16 * 1024 diff --git a/hermes_cli/auth_xai.py b/hermes_cli/auth_xai.py index 92b546cea2..1b43082b3e 100644 --- a/hermes_cli/auth_xai.py +++ b/hermes_cli/auth_xai.py @@ -211,6 +211,19 @@ def _is_xai_origin_host(host: str) -> bool: return host == "x.ai" or host.endswith(".x.ai") +def _xai_url_problem(url: str) -> tuple[Optional[str], str]: + """``(problem, host)`` — problem is ``"scheme"``, ``"host"``, ``"origin"`` or None when *url* is HTTPS on x.ai.""" + parsed = urlparse(url) + host = (parsed.hostname or "").lower() + if parsed.scheme != "https": + return "scheme", host + if not host: + return "host", host + if not _is_xai_origin_host(host): + return "origin", host + return None, host + + def _xai_validate_oauth_endpoint(url: str, *, field: str) -> str: """Refuse any OIDC discovery endpoint that isn't HTTPS on the xAI origin. @@ -218,21 +231,20 @@ def _xai_validate_oauth_endpoint(url: str, *, field: str) -> str: ``token_endpoint`` that receives the refresh_token forever. Pinning scheme + host (RFC 8414 §2) removes that persistence. """ - parsed = urlparse(url) - if parsed.scheme != "https": - raise _xai_err(f"xAI OIDC discovery returned a non-HTTPS {field}: {url!r}.", "xai_discovery_invalid") - host = (parsed.hostname or "").lower() - if not host: - raise _xai_err(f"xAI OIDC discovery {field} is missing a hostname: {url!r}.", "xai_discovery_invalid") - if not _is_xai_origin_host(host): - raise _xai_err( + problem, host = _xai_url_problem(url) + if problem is None: + return url + message = { + "scheme": f"xAI OIDC discovery returned a non-HTTPS {field}: {url!r}.", + "host": f"xAI OIDC discovery {field} is missing a hostname: {url!r}.", + "origin": ( f"xAI OIDC discovery {field} host {host!r} is not on the xAI origin " f"(expected x.ai or a *.x.ai subdomain). Refusing to use a cached " f"endpoint that may have been substituted by a MITM during initial " - f"discovery; re-authenticate with `hermes model` to re-fetch.", - "xai_discovery_invalid", - ) - return url + f"discovery; re-authenticate with `hermes model` to re-fetch." + ), + }[problem] + raise _xai_err(message, "xai_discovery_invalid") def _xai_validate_inference_base_url(value: str, *, fallback: str) -> str: @@ -246,22 +258,21 @@ def _xai_validate_inference_base_url(value: str, *, fallback: str) -> str: if not candidate: return fallback try: - parsed = urlparse(candidate) + problem, host = _xai_url_problem(candidate) except Exception: logger.warning("Ignoring malformed xAI base_url override %r; using %s instead.", candidate, fallback) return fallback - if parsed.scheme != "https": + if problem is None: + return candidate + if problem == "scheme": logger.warning( "Refusing non-HTTPS xAI base_url override %r (xai-oauth bearer would " "be sent in cleartext); falling back to %s.", candidate, fallback, ) - return fallback - host = (parsed.hostname or "").lower() - if not host: + elif problem == "host": logger.warning("Ignoring xAI base_url override %r with no hostname; using %s instead.", candidate, fallback) - return fallback - if not _is_xai_origin_host(host): + else: logger.warning( "Refusing xAI base_url override %r — host %r is not on the xAI origin " "(expected x.ai or a *.x.ai subdomain). The xai-oauth bearer is only " @@ -269,8 +280,7 @@ def _xai_validate_inference_base_url(value: str, *, fallback: str) -> str: "the credential. Falling back to %s.", candidate, host, fallback, ) - return fallback - return candidate + return fallback def _xai_oauth_discovery(timeout_seconds: float = 15.0) -> Dict[str, str]: diff --git a/hermes_cli/auth_zai_kimi.py b/hermes_cli/auth_zai_kimi.py index 64ab2733d3..78b206a9cf 100644 --- a/hermes_cli/auth_zai_kimi.py +++ b/hermes_cli/auth_zai_kimi.py @@ -16,7 +16,6 @@ from hermes_cli.auth_constants import httpx # Log-record parity with the origin module (caplog tests pin "hermes_cli.auth"). logger = logging.getLogger("hermes_cli.auth") - # Kimi Code (kimi.com/code) issues "sk-kimi-" keys that only work on api.kimi.com/coding; legacy # platform.moonshot.ai keys work on api.moonshot.ai/v1 (the old default). Intentionally NO /v1 # suffix: the /coding endpoint speaks Anthropic Messages and the SDK appends "/v1/messages" From f8e71375a791d42ac2a23a635f256a89d3c5150a Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 21:18:56 -0700 Subject: [PATCH 11/16] =?UTF-8?q?refactor(hermes=5Fcli):=20spotify=20?= =?UTF-8?q?=E2=80=94=20inline=20single-use=20exchange/scope=20helpers;=20m?= =?UTF-8?q?inimax=20=E2=80=94=20simpler=20bounded=20body=20reader;=20drop?= =?UTF-8?q?=20post-import=20blanks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/approval_mode.py | 2 -- hermes_cli/approvals_suggest.py | 6 ----- hermes_cli/approvals_test.py | 1 - hermes_cli/auth_minimax.py | 28 ++++++------------- hermes_cli/auth_model_picker.py | 7 ----- hermes_cli/auth_spotify.py | 48 +++++++++++---------------------- hermes_cli/auth_xai.py | 3 --- hermes_cli/auth_zai_kimi.py | 1 - hermes_cli/blueprint_cmd.py | 7 ----- 9 files changed, 23 insertions(+), 80 deletions(-) diff --git a/hermes_cli/approval_mode.py b/hermes_cli/approval_mode.py index 8e835e0b3d..6f019bbd01 100644 --- a/hermes_cli/approval_mode.py +++ b/hermes_cli/approval_mode.py @@ -27,7 +27,6 @@ class ApprovalModeResult: def _effective_mode() -> str: """Return the exact mode enforced by the terminal approval guard.""" from tools.approval import _get_approval_mode - return _get_approval_mode() @@ -46,7 +45,6 @@ def run_approval_mode_command(requested_mode: Optional[str]) -> ApprovalModeResu # unparseable config.yaml; capture both for slash-command output instead of terminating the # interactive worker. from hermes_cli.config import set_config_value - output = StringIO() try: with redirect_stdout(output), redirect_stderr(output): diff --git a/hermes_cli/approvals_suggest.py b/hermes_cli/approvals_suggest.py index 0dcd67f764..245cfc6c98 100644 --- a/hermes_cli/approvals_suggest.py +++ b/hermes_cli/approvals_suggest.py @@ -112,7 +112,6 @@ class Proposal: def default_db_path() -> Path: from hermes_constants import get_hermes_home - return get_hermes_home() / "state.db" @@ -175,7 +174,6 @@ def scan_approval_history(db_path: Optional[Path] = None, days: int = 90) -> lis that actually executed (i.e. carried an implied user approval). """ from tools.approval import detect_dangerous_command, detect_hardline_command - path = Path(db_path) if db_path else default_db_path() if not path.exists(): return [] @@ -208,7 +206,6 @@ def scan_approval_history(db_path: Optional[Path] = None, days: int = 90) -> lis def normalize_command(command: str) -> str: """Fold user/hermes home prefixes and collapse whitespace.""" from tools.approval import _rewrite_resolved_hermes_home, _rewrite_resolved_user_home - return " ".join(_rewrite_resolved_user_home(_rewrite_resolved_hermes_home(command)).split()) @@ -229,7 +226,6 @@ def derive_glob(normalized: str) -> Optional[str]: those anyway) and for commands anchored on an unsafe root binary. """ from tools.approval import _has_allowlist_shell_operator - tokens = normalized.split() if _has_allowlist_shell_operator(normalized) or not tokens or _unsafe_root_binary(tokens[0]): return None @@ -298,7 +294,6 @@ def parse_apply_indices(spec: str, total: int) -> list[int]: def apply_proposals(proposals: list[Proposal], indices: list[int]) -> set: """Merge chosen proposal patterns into command_allowlist and persist.""" import tools.approval as approval_module - merged = set(approval_module.load_permanent_allowlist()) | {proposals[idx].pattern for idx in indices} approval_module.save_permanent_allowlist(merged) # Keep the in-process allowlist consistent so a long-lived process sees the new entries @@ -341,7 +336,6 @@ def suggest_command(args) -> int: return 1 import tools.approval as approval_module - existing = set(approval_module.load_permanent_allowlist()) proposals = build_proposals( scan_approval_history(db_path, days=days), existing=existing, diff --git a/hermes_cli/approvals_test.py b/hermes_cli/approvals_test.py index 6400c61cab..cf822b5c6a 100644 --- a/hermes_cli/approvals_test.py +++ b/hermes_cli/approvals_test.py @@ -35,7 +35,6 @@ def evaluate_command(command: str, env_type: str = "local") -> dict: de-obfuscated forms the detectors actually evaluated). """ import tools.approval as approval - # Sync config-persisted "always" patterns so the allowlist check below sees what the runtime # would see (load is read-only). try: diff --git a/hermes_cli/auth_minimax.py b/hermes_cli/auth_minimax.py index 95b51e3fb5..dcccaaa914 100644 --- a/hermes_cli/auth_minimax.py +++ b/hermes_cli/auth_minimax.py @@ -23,7 +23,6 @@ from hermes_cli.auth_constants import ( if TYPE_CHECKING: # annotation-only; the runtime import would be a cycle from hermes_cli.auth import ProviderConfig - # Log-record parity with the origin module (caplog tests pin "hermes_cli.auth"). logger = logging.getLogger("hermes_cli.auth") @@ -33,34 +32,23 @@ _MINIMAX_OAUTH_ERROR_BODY_LIMIT = 16 * 1024 def _minimax_response_error_text(response: httpx.Response, *, limit: int = _MINIMAX_OAUTH_ERROR_BODY_LIMIT) -> str: """Return a bounded error body from a streamed MiniMax OAuth response.""" limit = max(0, int(limit)) - chunks: list[bytes] = [] - total = 0 - truncated = False try: if getattr(response, "is_stream_consumed", False): text = response.text return text[:limit] + ("...[truncated]" if len(text) > limit else "") - + # Read at most limit+1 bytes so truncation can be detected without buffering the whole body. + chunks: list[bytes] = [] + total = 0 for chunk in response.iter_bytes(): if not chunk: continue - remaining = limit + 1 - total - if remaining <= 0: - truncated = True + chunks.append(chunk[: limit + 1 - total]) + total += len(chunks[-1]) + if total > limit: break - if len(chunk) > remaining: - chunks.append(chunk[:remaining]) - total += remaining - truncated = True - break - chunks.append(chunk) - total += len(chunk) raw = b"".join(chunks) - if len(raw) > limit: - raw = raw[:limit] - truncated = True - text = raw.decode(response.encoding or "utf-8", errors="replace") - return text + ("...[truncated]" if truncated else "") + text = raw[:limit].decode(response.encoding or "utf-8", errors="replace") + return text + ("...[truncated]" if len(raw) > limit else "") finally: response.close() diff --git a/hermes_cli/auth_model_picker.py b/hermes_cli/auth_model_picker.py index 36616ffea9..e8e4ad49e2 100644 --- a/hermes_cli/auth_model_picker.py +++ b/hermes_cli/auth_model_picker.py @@ -31,7 +31,6 @@ def _confirm_selection_guards( """ try: from hermes_cli.model_selection_guards import combined_message, selection_warnings - warnings = selection_warnings( model_id, provider=provider, base_url=base_url, api_key=api_key, include_kinds=include_kinds, ) @@ -64,7 +63,6 @@ class _ModelPickerRows: current_model: str, sale_chrome: bool, ) -> None: from hermes_cli.models import _format_price_per_mtok, compute_sale_discount - self.current_model = current_model self.has_pricing = bool(pricing and any(pricing.get(m) for m in all_models)) # Leave room for a leading "★ " on sale rows (Nous only). @@ -165,7 +163,6 @@ def _prompt_model_selection( *unavailable_models* render grayed out and unselectable with an upgrade link to *portal_url*. """ from hermes_cli.cli_output import line_input - _unavailable = unavailable_models or [] # Sale chrome (★ / -N% / was) is Nous Portal-only — never for OpenRouter or other providers # even if pricing.original is somehow present. @@ -206,7 +203,6 @@ def _prompt_model_selection( # Try arrow-key menu first, fall back to number input. try: from hermes_cli.curses_ui import curses_radiolist - choices = [rows.segments(mid) for mid in ordered] + [_CUSTOM_LABEL, _SKIP_LABEL] unavailable_footer = unavailable_message.strip() @@ -229,7 +225,6 @@ def _prompt_model_selection( # ids (e.g. Kimi Coding `k3` ↔ query "kimi"). model_search_text always starts with the # wire id; only append when aliases add tokens beyond the bare id already in the label. from hermes_cli.model_search import model_search_text - model_search_labels = [] for mid in ordered: label, haystack = rows.label(mid), model_search_text(mid) @@ -259,7 +254,6 @@ def _prompt_model_selection( # Fallback: numbered list (ANSI colors for sale chrome) from hermes_cli.curses_ui import format_radio_item_ansi from hermes_cli.colors import Colors, color - for line in menu_title.splitlines(): print(line.replace("★", color("★", Colors.YELLOW), 1) if "★" in line else line) num_width = len(str(n + 2)) @@ -301,7 +295,6 @@ def _prompt_model_selection( def _save_model_choice(model_id: str) -> None: """Save the selected model to config.yaml only — NOT .env, which would stomp in multi-agent setups.""" from hermes_cli.config import save_config, load_config - config = load_config() # Always use dict format so provider/base_url can be stored alongside if isinstance(config.get("model"), dict): diff --git a/hermes_cli/auth_spotify.py b/hermes_cli/auth_spotify.py index bbf56e4672..370aa96545 100644 --- a/hermes_cli/auth_spotify.py +++ b/hermes_cli/auth_spotify.py @@ -18,7 +18,7 @@ import uuid import webbrowser from datetime import datetime, timezone from http.server import BaseHTTPRequestHandler, HTTPServer -from typing import Any, Dict, List, Optional, Tuple +from typing import Any, Dict, Optional, Tuple from urllib.parse import parse_qs, urlencode, urlparse from hermes_cli.auth_constants import ( AuthError, DEFAULT_SPOTIFY_ACCOUNTS_BASE_URL, DEFAULT_SPOTIFY_API_BASE_URL, DEFAULT_SPOTIFY_REDIRECT_URI, @@ -36,12 +36,9 @@ def _clean(value: Any) -> str: return str(value or "").strip() -def _spotify_scope_list(raw_scope: Optional[str] = None) -> List[str]: - return list(dict.fromkeys((raw_scope or DEFAULT_SPOTIFY_SCOPE).split())) - - def _spotify_scope_string(raw_scope: Optional[str] = None) -> str: - return " ".join(_spotify_scope_list(raw_scope)) + """Requested scope, whitespace-normalized and de-duplicated (order kept).""" + return " ".join(dict.fromkeys((raw_scope or DEFAULT_SPOTIFY_SCOPE).split())) def _spotify_setting( @@ -50,7 +47,6 @@ def _spotify_setting( ) -> str: """First non-empty of explicit arg, env vars (``.env`` aware), stored state, then *default*.""" from hermes_cli.config import get_env_value - candidates = ( explicit, *(get_env_value(var) for var in env_vars), state.get(state_key) if isinstance(state, dict) else None, default, @@ -246,27 +242,6 @@ def _spotify_token_post( return payload -def _spotify_exchange_code_for_tokens( - *, client_id: str, code: str, redirect_uri: str, code_verifier: str, accounts_base_url: str, - timeout_seconds: float = 20.0, -) -> Dict[str, Any]: - return _spotify_token_post( - accounts_base_url, - { - "client_id": client_id, - "grant_type": "authorization_code", - "code": code, - "redirect_uri": redirect_uri, - "code_verifier": code_verifier, - }, - timeout_seconds=timeout_seconds, - what="token exchange", - failed_code="spotify_token_exchange_failed", - invalid_code="spotify_token_exchange_invalid", - invalid_message="Spotify token response did not include an access_token.", - ) - - def _refresh_spotify_oauth_state(state: Dict[str, Any], *, timeout_seconds: float = 20.0) -> Dict[str, Any]: refresh_token = _clean(state.get("refresh_token")) if not refresh_token: @@ -370,7 +345,6 @@ def _spotify_interactive_setup(redirect_uri_hint: str) -> str: """Walk the user through creating a Spotify developer app; persist the client_id to ~/.hermes/.env.""" from hermes_cli.auth import _is_remote_session from hermes_cli.config import save_env_value - print( f"\n{'=' * 70}\nSpotify first-time setup\n{'=' * 70}\n\n" "Spotify requires every user to register their own lightweight\n" @@ -396,7 +370,6 @@ def _spotify_interactive_setup(redirect_uri_hint: str) -> str: pass from hermes_cli.cli_output import line_input - try: raw = line_input("Spotify Client ID: ").strip() except (EOFError, KeyboardInterrupt): @@ -469,10 +442,19 @@ def login_spotify_command(args) -> None: if callback.get("state") != state_nonce: raise SystemExit("Spotify authorization failed: state mismatch.") - token_payload = _spotify_exchange_code_for_tokens( - client_id=client_id, code=str(callback.get("code") or ""), redirect_uri=redirect_uri, - code_verifier=code_verifier, accounts_base_url=accounts_base_url, + token_payload = _spotify_token_post( + accounts_base_url, + { + "client_id": client_id, + "grant_type": "authorization_code", + "code": str(callback.get("code") or ""), + "redirect_uri": redirect_uri, + "code_verifier": code_verifier, + }, timeout_seconds=float(getattr(args, "timeout", None) or 20.0), + what="token exchange", failed_code="spotify_token_exchange_failed", + invalid_code="spotify_token_exchange_invalid", + invalid_message="Spotify token response did not include an access_token.", ) spotify_state = _spotify_token_payload_to_state( token_payload, client_id=client_id, redirect_uri=redirect_uri, requested_scope=scope, diff --git a/hermes_cli/auth_xai.py b/hermes_cli/auth_xai.py index 1b43082b3e..ae5db55109 100644 --- a/hermes_cli/auth_xai.py +++ b/hermes_cli/auth_xai.py @@ -26,7 +26,6 @@ from utils import env_float if TYPE_CHECKING: # annotation-only; the runtime import would be a cycle from hermes_cli.auth import ProviderConfig - # Log-record parity with the origin module (caplog tests pin "hermes_cli.auth"). logger = logging.getLogger("hermes_cli.auth") @@ -436,7 +435,6 @@ def resolve_xai_oauth_runtime_credentials( refresh_skew_seconds: Optional[int] = None, ) -> Dict[str, Any]: from hermes_cli.auth import _auth_store_lock, _is_terminal_xai_oauth_refresh_error, _refresh_xai_oauth_tokens, _xai_oauth_discovery - def _view(data: Dict[str, Any]) -> tuple[Dict[str, Any], str, str, str, bool]: tokens = dict(data["tokens"]) access_token = _clean(tokens.get("access_token")) @@ -546,7 +544,6 @@ def _xai_oauth_poll_device_token( poll_interval: int, ) -> Dict[str, Any]: from hermes_cli.auth import _poll_device_token_generic - def _validate(payload: Dict[str, Any]) -> None: for field_name, article in (("access_token", "an"), ("refresh_token", "a")): if not payload.get(field_name): diff --git a/hermes_cli/auth_zai_kimi.py b/hermes_cli/auth_zai_kimi.py index 78b206a9cf..ff03a7b55d 100644 --- a/hermes_cli/auth_zai_kimi.py +++ b/hermes_cli/auth_zai_kimi.py @@ -69,7 +69,6 @@ def _probe_single_zai_endpoint(api_key: str, endpoint: tuple, timeout: float) -> def detect_zai_endpoint(api_key: str, timeout: float = 8.0) -> Optional[Dict[str, str]]: """Probe z.ai endpoints in parallel; first working one in ZAI_ENDPOINTS priority order, or None.""" from concurrent.futures import ThreadPoolExecutor, as_completed - # No `with` block: a context manager would join ALL probe threads on exit, defeating the early # return below. shutdown(wait=False) lets surviving probes drain in the background. pool = ThreadPoolExecutor(max_workers=len(ZAI_ENDPOINTS)) diff --git a/hermes_cli/blueprint_cmd.py b/hermes_cli/blueprint_cmd.py index 68a2413045..c177d1c75e 100644 --- a/hermes_cli/blueprint_cmd.py +++ b/hermes_cli/blueprint_cmd.py @@ -29,7 +29,6 @@ def _resolve_origin(explicit: Optional[Dict[str, Any]]) -> Optional[Dict[str, An return explicit try: from gateway.session_context import get_session_env - platform = get_session_env("HERMES_SESSION_PLATFORM") chat_id = get_session_env("HERMES_SESSION_CHAT_ID") if platform and chat_id: @@ -74,7 +73,6 @@ def match_blueprint(query: str) -> Tuple[Optional[Any], List[Any]]: anywhere in key/title/description, then a difflib fuzzy pass on keys. """ from cron.blueprint_catalog import CATALOG, get_blueprint - q = (query or "").strip().lower() if not q: return None, [] @@ -97,7 +95,6 @@ def match_blueprint(query: str) -> Tuple[Optional[Any], List[Any]]: def _humanize_schedule(blueprint) -> str: from cron.blueprint_catalog import _humanize_schedule as _h - try: return _h(blueprint) except Exception: @@ -112,7 +109,6 @@ def build_blueprint_seed(blueprint) -> str: rendered prompt. Defaults are stated so the agent can offer them. """ from cron.blueprint_catalog import WEEKDAY_PRESETS - lines: List[str] = [ f"Set up the '{blueprint.title}' automation for me (automation blueprint " f"'{blueprint.key}'). {blueprint.description}", @@ -148,7 +144,6 @@ def build_blueprint_seed(blueprint) -> str: def _fmt_catalog() -> str: from cron.blueprint_catalog import CATALOG - lines = ["Automation Blueprints — `/blueprint ` and I'll ask you what I need:\n"] for r in CATALOG: lines.append(f" • {r.key} — {r.title}") @@ -169,7 +164,6 @@ def _fmt_candidates(query: str, candidates: List[Any]) -> str: def _fmt_no_match(query: str) -> str: from cron.blueprint_catalog import CATALOG - close = difflib.get_close_matches((query or "").lower(), [r.key for r in CATALOG], n=3, cutoff=0.4) msg = f"No automation blueprint matches '{query}'." if close: @@ -229,7 +223,6 @@ def handle_blueprint_command( try: from cron.scheduler import CronSchedulerRegistrationError, create_job_with_scheduler_registration - job = create_job_with_scheduler_registration(**spec) except CronSchedulerRegistrationError as e: return BlueprintCommandResult(e.user_message()) From 7010f2d996987c7b452d68837660377a535c7932 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 21:26:16 -0700 Subject: [PATCH 12/16] =?UTF-8?q?refactor(hermes=5Fcli):=20group=20C=20?= =?UTF-8?q?=E2=80=94=20pack=20short=20dict/kwarg=20spans=20(AST-verified)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/approvals_suggest.py | 8 ++----- hermes_cli/approvals_test.py | 12 +++------- hermes_cli/auth_minimax.py | 14 ++++------- hermes_cli/auth_qwen.py | 11 +++------ hermes_cli/auth_spotify.py | 42 ++++++++++----------------------- hermes_cli/auth_xai.py | 19 +++++---------- hermes_cli/auth_zai_kimi.py | 6 ++--- hermes_cli/blueprint_cmd.py | 3 +-- 8 files changed, 34 insertions(+), 81 deletions(-) diff --git a/hermes_cli/approvals_suggest.py b/hermes_cli/approvals_suggest.py index 245cfc6c98..a09225d567 100644 --- a/hermes_cli/approvals_suggest.py +++ b/hermes_cli/approvals_suggest.py @@ -367,12 +367,8 @@ def suggest_command(args) -> int: "days": days, "proposals": [ { - "n": i, - "pattern": p.pattern, - "kind": p.kind, - "count": p.count, - "classes": sorted(p.classes), - "examples": p.examples, + "n": i, "pattern": p.pattern, "kind": p.kind, "count": p.count, + "classes": sorted(p.classes), "examples": p.examples, } for i, p in enumerate(proposals, 1) ], diff --git a/hermes_cli/approvals_test.py b/hermes_cli/approvals_test.py index cf822b5c6a..495732ebb6 100644 --- a/hermes_cli/approvals_test.py +++ b/hermes_cli/approvals_test.py @@ -20,9 +20,7 @@ EXIT_ASK = 2 EXIT_DENY = 3 _VERDICT_EXIT = { - "allow": EXIT_ALLOW, - "ask-approval": EXIT_ASK, - "hardline-deny": EXIT_DENY, + "allow": EXIT_ALLOW, "ask-approval": EXIT_ASK, "hardline-deny": EXIT_DENY, "user-deny": EXIT_DENY, } @@ -46,12 +44,8 @@ def evaluate_command(command: str, env_type: str = "local") -> dict: def result(verdict: str, rule=None, detail: str = "") -> dict: return { - "command": command, - "env_type": env_type, - "verdict": verdict, - "exit_code": _VERDICT_EXIT[verdict], - "rule": rule, - "detail": detail, + "command": command, "env_type": env_type, "verdict": verdict, + "exit_code": _VERDICT_EXIT[verdict], "rule": rule, "detail": detail, "normalized_variants": variants, } diff --git a/hermes_cli/auth_minimax.py b/hermes_cli/auth_minimax.py index dcccaaa914..b3aa9e6281 100644 --- a/hermes_cli/auth_minimax.py +++ b/hermes_cli/auth_minimax.py @@ -76,12 +76,8 @@ def _minimax_request_user_code( client, f"{portal_base_url}/oauth/code", data={ - "response_type": "code", - "client_id": client_id, - "scope": MINIMAX_OAUTH_SCOPE, - "code_challenge": code_challenge, - "code_challenge_method": "S256", - "state": state, + "response_type": "code", "client_id": client_id, "scope": MINIMAX_OAUTH_SCOPE, + "code_challenge": code_challenge, "code_challenge_method": "S256", "state": state, }, headers={**_FORM_JSON_HEADERS, "x-request-id": str(uuid.uuid4())}, ) @@ -135,10 +131,8 @@ def _minimax_poll_token( client, f"{portal_base_url}/oauth/token", data={ - "grant_type": MINIMAX_OAUTH_GRANT_TYPE, - "client_id": client_id, - "user_code": user_code, - "code_verifier": code_verifier, + "grant_type": MINIMAX_OAUTH_GRANT_TYPE, "client_id": client_id, + "user_code": user_code, "code_verifier": code_verifier, }, headers=_FORM_JSON_HEADERS, ) diff --git a/hermes_cli/auth_qwen.py b/hermes_cli/auth_qwen.py index ad15b86300..849dbfd066 100644 --- a/hermes_cli/auth_qwen.py +++ b/hermes_cli/auth_qwen.py @@ -137,9 +137,7 @@ def resolve_qwen_runtime_credentials( return { "provider": "qwen-oauth", "base_url": os.getenv("HERMES_QWEN_BASE_URL", "").strip().rstrip("/") or DEFAULT_QWEN_BASE_URL, - "api_key": access_token, - "source": "qwen-cli", - "expires_at_ms": tokens.get("expiry_date"), + "api_key": access_token, "source": "qwen-cli", "expires_at_ms": tokens.get("expiry_date"), "auth_file": str(_qwen_cli_auth_path()), } @@ -153,11 +151,8 @@ def get_qwen_auth_status() -> Dict[str, Any]: # broken Qwen setup flow. creds = resolve_qwen_runtime_credentials(refresh_if_expiring=True) return { - "logged_in": True, - "auth_file": str(auth_path), - "source": creds.get("source"), - "api_key": creds.get("api_key"), - "expires_at_ms": creds.get("expires_at_ms"), + "logged_in": True, "auth_file": str(auth_path), "source": creds.get("source"), + "api_key": creds.get("api_key"), "expires_at_ms": creds.get("expires_at_ms"), } except AuthError as exc: return {"logged_in": False, "auth_file": str(auth_path), "error": str(exc)} diff --git a/hermes_cli/auth_spotify.py b/hermes_cli/auth_spotify.py index 370aa96545..5131bc7851 100644 --- a/hermes_cli/auth_spotify.py +++ b/hermes_cli/auth_spotify.py @@ -106,12 +106,8 @@ def _spotify_build_authorize_url( accounts_base_url: str, ) -> str: query = urlencode({ - "client_id": client_id, - "response_type": "code", - "redirect_uri": redirect_uri, - "scope": scope, - "state": state, - "code_challenge_method": "S256", + "client_id": client_id, "response_type": "code", "redirect_uri": redirect_uri, + "scope": scope, "state": state, "code_challenge_method": "S256", "code_challenge": code_challenge, }) return f"{accounts_base_url}/authorize?{query}" @@ -197,19 +193,15 @@ def _spotify_token_payload_to_state( expires_at = datetime.fromtimestamp(now.timestamp() + expires_in, tz=timezone.utc) state = dict(previous_state or {}) state.update({ - "client_id": client_id, - "redirect_uri": redirect_uri, - "accounts_base_url": accounts_base_url, - "api_base_url": api_base_url, + "client_id": client_id, "redirect_uri": redirect_uri, + "accounts_base_url": accounts_base_url, "api_base_url": api_base_url, "scope": requested_scope, "granted_scope": str(token_payload.get("scope") or requested_scope).strip(), "token_type": _clean(token_payload.get("token_type", "Bearer") or "Bearer") or "Bearer", "access_token": _clean(token_payload.get("access_token")), "refresh_token": _clean(token_payload.get("refresh_token") or state.get("refresh_token")), - "obtained_at": now.isoformat(), - "expires_at": expires_at.isoformat(), - "expires_in": expires_in, - "auth_type": "oauth_pkce", + "obtained_at": now.isoformat(), "expires_at": expires_at.isoformat(), + "expires_in": expires_in, "auth_type": "oauth_pkce", }) return state @@ -308,15 +300,12 @@ def resolve_spotify_runtime_credentials( ) return { - "provider": "spotify", - "access_token": access_token, - "api_key": access_token, + "provider": "spotify", "access_token": access_token, "api_key": access_token, "token_type": str(state.get("token_type", "Bearer") or "Bearer"), "base_url": _spotify_api_base_url(state), "scope": _clean(state.get("granted_scope") or state.get("scope")), "client_id": _spotify_client_id(state=state), - "redirect_uri": _spotify_redirect_uri(state=state), - "expires_at": state.get("expires_at"), + "redirect_uri": _spotify_redirect_uri(state=state), "expires_at": state.get("expires_at"), "refresh_token": _clean(state.get("refresh_token")), } @@ -331,13 +320,10 @@ def get_spotify_auth_status() -> Dict[str, Any]: refresh_token = _clean(state.get("refresh_token")) return { "logged_in": bool(refresh_token or not _is_expiring(expires_at, 0)), - "auth_type": state.get("auth_type", "oauth_pkce"), - "client_id": state.get("client_id"), + "auth_type": state.get("auth_type", "oauth_pkce"), "client_id": state.get("client_id"), "redirect_uri": state.get("redirect_uri"), - "scope": state.get("granted_scope") or state.get("scope"), - "expires_at": expires_at, - "api_base_url": state.get("api_base_url"), - "has_refresh_token": bool(refresh_token), + "scope": state.get("granted_scope") or state.get("scope"), "expires_at": expires_at, + "api_base_url": state.get("api_base_url"), "has_refresh_token": bool(refresh_token), } @@ -445,10 +431,8 @@ def login_spotify_command(args) -> None: token_payload = _spotify_token_post( accounts_base_url, { - "client_id": client_id, - "grant_type": "authorization_code", - "code": str(callback.get("code") or ""), - "redirect_uri": redirect_uri, + "client_id": client_id, "grant_type": "authorization_code", + "code": str(callback.get("code") or ""), "redirect_uri": redirect_uri, "code_verifier": code_verifier, }, timeout_seconds=float(getattr(args, "timeout", None) or 20.0), diff --git a/hermes_cli/auth_xai.py b/hermes_cli/auth_xai.py index ae5db55109..4286c87b5f 100644 --- a/hermes_cli/auth_xai.py +++ b/hermes_cli/auth_xai.py @@ -58,8 +58,7 @@ def _xai_oauth_state_from_store(auth_store: Dict[str, Any]) -> Optional[Dict[str continue merged = dict(state or {}) merged["tokens"] = { - "access_token": access_token, - "refresh_token": refresh_token, + "access_token": access_token, "refresh_token": refresh_token, "token_type": str(entry.get("token_type") or "Bearer"), } if entry.get("last_refresh"): @@ -96,10 +95,8 @@ def _read_xai_oauth_tokens(*, _lock: bool = True) -> Dict[str, Any]: f"xAI OAuth state is missing {field}. {_RELOGIN}", f"xai_auth_missing_{field}", relogin=True, ) return { - "tokens": tokens, - "last_refresh": state.get("last_refresh"), - "discovery": state.get("discovery") or {}, - "redirect_uri": state.get("redirect_uri"), + "tokens": tokens, "last_refresh": state.get("last_refresh"), + "discovery": state.get("discovery") or {}, "redirect_uri": state.get("redirect_uri"), } @@ -308,8 +305,7 @@ def _xai_tokens_from_payload(payload: Dict[str, Any], access_token: str, fallbac return { "access_token": access_token, "refresh_token": str(payload.get("refresh_token") or fallback_refresh).strip(), - "id_token": _clean(payload.get("id_token")), - "expires_in": payload.get("expires_in"), + "id_token": _clean(payload.get("id_token")), "expires_in": payload.get("expires_in"), "token_type": _clean(payload.get("token_type") or "Bearer") or "Bearer", } @@ -597,9 +593,6 @@ def _xai_oauth_device_code_login(*, timeout_seconds: float = 20.0, open_browser: raise _xai_err("xAI device-code token response was missing required tokens.", "xai_device_token_invalid") return { "tokens": _xai_tokens_from_payload(payload, access_token, refresh_token), - "discovery": discovery, - "redirect_uri": "", - "base_url": _xai_oauth_inference_base_url(), - "last_refresh": _utc_now_z(), - "source": "oauth-device-code", + "discovery": discovery, "redirect_uri": "", "base_url": _xai_oauth_inference_base_url(), + "last_refresh": _utc_now_z(), "source": "oauth-device-code", } diff --git a/hermes_cli/auth_zai_kimi.py b/hermes_cli/auth_zai_kimi.py index ff03a7b55d..c343cc696d 100644 --- a/hermes_cli/auth_zai_kimi.py +++ b/hermes_cli/auth_zai_kimi.py @@ -130,10 +130,8 @@ def _resolve_zai_base_url(api_key: str, default_url: str, env_override: str) -> return default_url detected_endpoint = { - "base_url": detected["base_url"], - "endpoint_id": detected.get("id", ""), - "model": detected.get("model", ""), - "label": detected.get("label", ""), + "base_url": detected["base_url"], "endpoint_id": detected.get("id", ""), + "model": detected.get("model", ""), "label": detected.get("label", ""), "key_hash": key_hash, } # Persist failure (disk full, permissions, lock timeout) must not break resolution — detection diff --git a/hermes_cli/blueprint_cmd.py b/hermes_cli/blueprint_cmd.py index c177d1c75e..778d801bae 100644 --- a/hermes_cli/blueprint_cmd.py +++ b/hermes_cli/blueprint_cmd.py @@ -33,8 +33,7 @@ def _resolve_origin(explicit: Optional[Dict[str, Any]]) -> Optional[Dict[str, An chat_id = get_session_env("HERMES_SESSION_CHAT_ID") if platform and chat_id: return { - "platform": platform, - "chat_id": chat_id, + "platform": platform, "chat_id": chat_id, "chat_name": get_session_env("HERMES_SESSION_CHAT_NAME") or None, "thread_id": get_session_env("HERMES_SESSION_THREAD_ID") or None, } From 3f7edcd8ef854357f9986b4df611286c7e8a572c Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 21:48:05 -0700 Subject: [PATCH 13/16] =?UTF-8?q?refactor(hermes=5Fcli):=20auth=5Fxai=20?= =?UTF-8?q?=E2=80=94=20resolve=5Fxai=5Foauth=5Fruntime=5Fcredentials:=20si?= =?UTF-8?q?ngle=20should-refresh=20predicate,=20no=205-tuple=20view?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/approvals_suggest.py | 6 ++---- hermes_cli/auth_xai.py | 35 ++++++++++++++++----------------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/hermes_cli/approvals_suggest.py b/hermes_cli/approvals_suggest.py index a09225d567..3965414210 100644 --- a/hermes_cli/approvals_suggest.py +++ b/hermes_cli/approvals_suggest.py @@ -135,8 +135,7 @@ def _json_or_none(raw): def _iter_terminal_calls(con: sqlite3.Connection, since_ts: float) -> Iterator[tuple[str, str]]: """Yield ``(tool_call_id, command)`` for every terminal tool call.""" cur = con.execute( - "SELECT tool_calls FROM messages " - "WHERE role='assistant' AND tool_calls IS NOT NULL " + "SELECT tool_calls FROM messages WHERE role='assistant' AND tool_calls IS NOT NULL " "AND tool_calls LIKE '%terminal%' AND timestamp >= ?", (since_ts,), ) @@ -308,8 +307,7 @@ def _render_text(proposals: list[Proposal], days: int) -> None: print( f"No allowlist candidates found in approval history ({window}).\n" "Either nothing dangerous was approved often enough " - "(see --min-count/--days), or the approved classes are " - "excluded for safety." + "(see --min-count/--days), or the approved classes are excluded for safety." ) return print(f"Proposed command_allowlist additions (from approval history, {window}):\n") diff --git a/hermes_cli/auth_xai.py b/hermes_cli/auth_xai.py index 4286c87b5f..03c248b29f 100644 --- a/hermes_cli/auth_xai.py +++ b/hermes_cli/auth_xai.py @@ -431,36 +431,35 @@ def resolve_xai_oauth_runtime_credentials( refresh_skew_seconds: Optional[int] = None, ) -> Dict[str, Any]: from hermes_cli.auth import _auth_store_lock, _is_terminal_xai_oauth_refresh_error, _refresh_xai_oauth_tokens, _xai_oauth_discovery - def _view(data: Dict[str, Any]) -> tuple[Dict[str, Any], str, str, str, bool]: - tokens = dict(data["tokens"]) - access_token = _clean(tokens.get("access_token")) - token_endpoint = _clean(dict(data.get("discovery") or {}).get("token_endpoint")) - redirect_uri = _clean(data.get("redirect_uri")) - effective_skew = ( + + def _should_refresh(data: Dict[str, Any]) -> bool: + access_token = _clean(data["tokens"].get("access_token")) + skew = ( int(refresh_skew_seconds) if refresh_skew_seconds is not None else _xai_proactive_refresh_skew_seconds(access_token) ) - should_refresh = bool(force_refresh) or bool( - refresh_if_expiring and _xai_access_token_is_expiring(access_token, effective_skew) + return bool(force_refresh) or bool( + refresh_if_expiring and _xai_access_token_is_expiring(access_token, skew) ) - return tokens, access_token, token_endpoint, redirect_uri, should_refresh data = _read_xai_oauth_tokens() + tokens = dict(data["tokens"]) refresh_timeout_seconds = env_float("HERMES_XAI_REFRESH_TIMEOUT_SECONDS", 20) - tokens, access_token, token_endpoint, redirect_uri, should_refresh = _view(data) - if should_refresh: + if _should_refresh(data): with _auth_store_lock(timeout_seconds=max(float(AUTH_LOCK_TIMEOUT_SECONDS), refresh_timeout_seconds + 5.0)): + # Re-read under the lock: a concurrent caller may already have rotated the grant. data = _read_xai_oauth_tokens(_lock=False) - tokens, access_token, token_endpoint, redirect_uri, should_refresh = _view(data) - if should_refresh: - if not token_endpoint: - token_endpoint = _xai_oauth_discovery(refresh_timeout_seconds)["token_endpoint"] + tokens = dict(data["tokens"]) + if _should_refresh(data): + token_endpoint = ( + _clean(dict(data.get("discovery") or {}).get("token_endpoint")) + or _xai_oauth_discovery(refresh_timeout_seconds)["token_endpoint"] + ) try: tokens = _refresh_xai_oauth_tokens( - tokens, token_endpoint=token_endpoint, redirect_uri=redirect_uri, + tokens, token_endpoint=token_endpoint, redirect_uri=_clean(data.get("redirect_uri")), timeout_seconds=refresh_timeout_seconds, ) - access_token = _clean(tokens.get("access_token")) except AuthError as exc: if _is_terminal_xai_oauth_refresh_error(exc): _quarantine_xai_oauth_tokens(exc) @@ -469,7 +468,7 @@ def resolve_xai_oauth_runtime_credentials( return { "provider": "xai-oauth", "base_url": _xai_oauth_inference_base_url(), - "api_key": access_token, + "api_key": _clean(tokens.get("access_token")), "source": "hermes-auth-store", "last_refresh": data.get("last_refresh"), # Display/telemetry only. Device-code is the only supported xAI OAuth flow, so report it From 105c4413d3e6dc2d9037a136b02107f8bdd0723f Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 21:55:44 -0700 Subject: [PATCH 14/16] =?UTF-8?q?refactor(hermes=5Fcli):=20auth=5Fmodel=5F?= =?UTF-8?q?picker=20=E2=80=94=20reuse=20custom-entry=20helper=20in=20fallb?= =?UTF-8?q?ack;=20single-expr=20header=20lines?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/auth_model_picker.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/hermes_cli/auth_model_picker.py b/hermes_cli/auth_model_picker.py index e8e4ad49e2..cda7693f0c 100644 --- a/hermes_cli/auth_model_picker.py +++ b/hermes_cli/auth_model_picker.py @@ -212,11 +212,7 @@ def _prompt_model_selection( # The pricing column header (and any unavailable-models block) is shown as a multi-line # description above the list so it survives the curses screen clear. menu_title already # embeds the aligned price header; keep only the header/legend portion. - desc_lines: list[str] = [] - if rows.has_pricing: - header_part = menu_title.split("\n", 1) - if len(header_part) > 1: - desc_lines.extend(header_part[1].splitlines()) + desc_lines: list[str] = menu_title.split("\n", 1)[1].splitlines() if rows.has_pricing else [] if _unavailable: desc_lines.extend(f" {rows.label(mid)}" for mid in _unavailable) desc_lines.append(f" ── {unavailable_footer} ──") @@ -281,8 +277,7 @@ def _prompt_model_selection( if 1 <= idx <= n: return _confirmed_selection(ordered[idx - 1]) if idx == n + 1: - custom = line_input("Enter model name: ").strip() - return _confirmed_selection(custom) if custom else None + return _custom_selection() if idx == n + 2: return None print(f"Please enter 1-{n + 2}") From 16579014cb6da54b8ee793cd16549ddf3b396474 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 21:59:38 -0700 Subject: [PATCH 15/16] =?UTF-8?q?refactor(hermes=5Fcli):=20group=20C=20aut?= =?UTF-8?q?h=20modules=20=E2=80=94=20compact=20WHY-preserving=20comments?= =?UTF-8?q?=20and=20docstrings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/auth_minimax.py | 28 ++++------ hermes_cli/auth_model_picker.py | 28 ++++------ hermes_cli/auth_qwen.py | 18 ++----- hermes_cli/auth_spotify.py | 13 ++--- hermes_cli/auth_xai.py | 92 +++++++++++++-------------------- hermes_cli/auth_zai_kimi.py | 32 ++++-------- 6 files changed, 73 insertions(+), 138 deletions(-) diff --git a/hermes_cli/auth_minimax.py b/hermes_cli/auth_minimax.py index b3aa9e6281..65ac3a2cd8 100644 --- a/hermes_cli/auth_minimax.py +++ b/hermes_cli/auth_minimax.py @@ -1,9 +1,7 @@ """MiniMax OAuth (user-code grant) login, refresh and runtime credentials. -Split out of ``hermes_cli/auth.py``; every moved name is re-imported there, so -``hermes_cli.auth.`` keeps resolving (and monkeypatching) as before. Origin-internal -helpers are imported lazily inside each function (no import cycle; patches on -``hermes_cli.auth.`` still intercept). +Re-exported from ``hermes_cli/auth.py`` (patch targets unchanged); origin helpers are imported +lazily per function so ``hermes_cli.auth.`` patches still intercept and no cycle forms. """ from __future__ import annotations @@ -23,7 +21,6 @@ from hermes_cli.auth_constants import ( if TYPE_CHECKING: # annotation-only; the runtime import would be a cycle from hermes_cli.auth import ProviderConfig -# Log-record parity with the origin module (caplog tests pin "hermes_cli.auth"). logger = logging.getLogger("hermes_cli.auth") _MINIMAX_OAUTH_ERROR_BODY_LIMIT = 16 * 1024 @@ -121,8 +118,7 @@ def _minimax_poll_token( client: httpx.Client, *, portal_base_url: str, client_id: str, user_code: str, code_verifier: str, expired_in: int, interval_ms: Optional[int], ) -> Dict[str, Any]: - # OpenClaw treats expired_in as a unix-ms timestamp; if it's small enough to be a duration, - # treat it as seconds. + # expired_in is a unix-ms timestamp upstream (OpenClaw) but small values are TTL seconds. deadline = _minimax_resolve_token_expiry_unix(expired_in, now=datetime.now(timezone.utc)) interval = max(2.0, (interval_ms or 2000) / 1000.0) @@ -247,9 +243,8 @@ def _refresh_minimax_oauth_state(state: Dict[str, Any], *, timeout_seconds: floa data={"grant_type": "refresh_token", "client_id": state["client_id"], "refresh_token": state["refresh_token"]}, headers=_FORM_JSON_HEADERS, ) - # The non-200 branch reads a STREAMED body, so it must run while the client is still open - # (iter_bytes() after close raises StreamClosed). The 200 path was already read by - # _minimax_post_form, so response.json() below is safe outside. + # Non-200 reads a STREAMED body, so it must run inside the client context (iter_bytes() + # after close raises StreamClosed); the 200 body was already read by _minimax_post_form. if response.status_code != 200: body = _minimax_response_error_text(response) body_lower = body.lower() @@ -298,11 +293,10 @@ def _minimax_fresh_state() -> Dict[str, Any]: def build_minimax_oauth_token_provider() -> Callable[[], str]: - """Return a zero-arg callable that yields a fresh MiniMax access token. + """Zero-arg callable yielding a fresh MiniMax access token. - The Anthropic SDK caches ``api_key`` as a static string at construction time, so a session that - resolves credentials once at startup would keep sending the same bearer until MiniMax returns - 401 — typically ~15 minutes in, because MiniMax issues short-lived access tokens. + The Anthropic SDK caches ``api_key`` at construction; MiniMax tokens live ~15 minutes, so a + static bearer would start 401-ing mid-session. """ def _provide() -> str: token = _minimax_fresh_state().get("access_token") @@ -317,11 +311,7 @@ def resolve_minimax_oauth_runtime_credentials( *, min_token_ttl_seconds: int = MINIMAX_OAUTH_REFRESH_SKEW_SECONDS, as_token_provider: bool = False, ) -> Dict[str, Any]: - """Return {provider, api_key, base_url, source} for minimax-oauth. - - The default (string ``api_key``) preserves the historical contract for diagnostic call sites - like ``hermes status`` that just want to know whether a valid token exists right now. - """ + """Return {provider, api_key, base_url, source}; string ``api_key`` by default (``hermes status`` contract).""" state = _minimax_fresh_state() return { "provider": "minimax-oauth", diff --git a/hermes_cli/auth_model_picker.py b/hermes_cli/auth_model_picker.py index cda7693f0c..181d7c5497 100644 --- a/hermes_cli/auth_model_picker.py +++ b/hermes_cli/auth_model_picker.py @@ -1,9 +1,7 @@ """Interactive model picker used after OAuth login. -Split out of ``hermes_cli/auth.py``; every moved name is re-imported there, so -``hermes_cli.auth.`` keeps resolving (and monkeypatching) as before. Origin-internal -helpers are imported lazily inside each function (no import cycle; patches on -``hermes_cli.auth.`` still intercept). +Re-exported from ``hermes_cli/auth.py`` (patch targets unchanged); origin helpers are imported +lazily per function so ``hermes_cli.auth.`` patches still intercept and no cycle forms. """ from __future__ import annotations @@ -13,7 +11,6 @@ import subprocess from typing import Dict, List, Optional from hermes_cli.auth_constants import DEFAULT_NOUS_PORTAL_URL -# Log-record parity with the origin module (caplog tests pin "hermes_cli.auth"). logger = logging.getLogger("hermes_cli.auth") _CUSTOM_LABEL = "Enter custom model name" @@ -52,10 +49,10 @@ def _confirm_selection_guards( class _ModelPickerRows: - """Column-aligned model rows (name + $/Mtok prices + Nous sale chrome) for the model picker. + """Column-aligned picker rows (name + $/Mtok + Nous sale chrome). - Sale chrome (★ / -N% / was) is drawn as curses/ANSI segments (yellow % / dim "was"), not baked - into one plain string — curses addnstr would otherwise render escape bytes literally. + Sale chrome is emitted as styled segments, not ANSI baked into one string — curses addnstr + would render escape bytes literally. """ def __init__( @@ -164,15 +161,13 @@ def _prompt_model_selection( """ from hermes_cli.cli_output import line_input _unavailable = unavailable_models or [] - # Sale chrome (★ / -N% / was) is Nous Portal-only — never for OpenRouter or other providers - # even if pricing.original is somehow present. + # Sale chrome is Nous Portal-only, even if pricing.original is present for another provider. sale_chrome = (confirm_provider or "").strip().lower() == "nous" def _confirmed_selection(mid: str) -> Optional[str]: if not mid: return None - # The cost guard only runs when a provider is known (pricing lookups need one); id-keyed - # guards like the data-policy guard always run — even via a custom endpoint or gateway. + # Cost guard needs a known provider; id-keyed guards (data policy) always run. ok = _confirm_selection_guards( mid, provider=confirm_provider, base_url=confirm_base_url, api_key=confirm_api_key, include_kinds=None if confirm_provider else ["data_policy"], @@ -209,17 +204,14 @@ def _prompt_model_selection( if not unavailable_footer and _unavailable: unavailable_footer = f"Upgrade at {_upgrade_url} for paid models" - # The pricing column header (and any unavailable-models block) is shown as a multi-line - # description above the list so it survives the curses screen clear. menu_title already - # embeds the aligned price header; keep only the header/legend portion. + # Header/legend + unavailable block go in the description so they survive the curses clear. desc_lines: list[str] = menu_title.split("\n", 1)[1].splitlines() if rows.has_pricing else [] if _unavailable: desc_lines.extend(f" {rows.label(mid)}" for mid in _unavailable) desc_lines.append(f" ── {unavailable_footer} ──") - # Search haystacks keep pricing labels visible while adding aliases for brand-less wire - # ids (e.g. Kimi Coding `k3` ↔ query "kimi"). model_search_text always starts with the - # wire id; only append when aliases add tokens beyond the bare id already in the label. + # Search haystack = label + aliases for brand-less wire ids (Kimi `k3` ↔ "kimi"); skip when + # model_search_text adds nothing beyond the bare id. from hermes_cli.model_search import model_search_text model_search_labels = [] for mid in ordered: diff --git a/hermes_cli/auth_qwen.py b/hermes_cli/auth_qwen.py index 849dbfd066..860a7e37a3 100644 --- a/hermes_cli/auth_qwen.py +++ b/hermes_cli/auth_qwen.py @@ -1,9 +1,7 @@ """Qwen OAuth (qwen-cli token file) runtime credentials and status. -Split out of ``hermes_cli/auth.py``; every moved name is re-imported there, so -``hermes_cli.auth.`` keeps resolving (and monkeypatching) as before. Origin-internal -helpers are imported lazily inside each function (no import cycle; patches on -``hermes_cli.auth.`` still intercept). +Re-exported from ``hermes_cli/auth.py`` (patch targets unchanged); origin helpers are imported +lazily per function so ``hermes_cli.auth.`` patches still intercept and no cycle forms. """ from __future__ import annotations @@ -19,7 +17,6 @@ from hermes_cli.auth_constants import ( QWEN_OAUTH_TOKEN_URL, _FORM_JSON_HEADERS, _qwen_err, httpx, ) -# Log-record parity with the origin module (caplog tests pin "hermes_cli.auth"). logger = logging.getLogger("hermes_cli.auth") _RERUN = "Re-run 'qwen auth qwen-oauth'." @@ -105,12 +102,7 @@ def _refresh_qwen_cli_tokens(tokens: Dict[str, Any], timeout_seconds: float = 20 def _mark_qwen_oauth_active(creds: Dict[str, Any]) -> None: - """Set active_provider to qwen-oauth in auth.json. - - Qwen tokens live in the Qwen CLI credential file, so this writes only a minimal provider-state - entry (base_url for display) and sets active_provider so ``get_active_provider()`` and the - setup wizard's credential check detect the provider. - """ + """Set active_provider to qwen-oauth with a minimal state entry (tokens stay in the Qwen CLI file).""" from hermes_cli.auth import _auth_store_lock, _load_auth_store, _save_auth_store, _save_provider_state with _auth_store_lock(): auth_store = _load_auth_store() @@ -146,9 +138,7 @@ def get_qwen_auth_status() -> Dict[str, Any]: from hermes_cli.auth import _qwen_cli_auth_path, resolve_qwen_runtime_credentials auth_path = _qwen_cli_auth_path() try: - # Validate the runtime credentials, including refresh when the cached CLI token is expired; - # otherwise stale tokens show up as "logged in" and `hermes model` walks users into a - # broken Qwen setup flow. + # Refresh-validate: otherwise stale CLI tokens read as "logged in" and break `hermes model`. creds = resolve_qwen_runtime_credentials(refresh_if_expiring=True) return { "logged_in": True, "auth_file": str(auth_path), "source": creds.get("source"), diff --git a/hermes_cli/auth_spotify.py b/hermes_cli/auth_spotify.py index 5131bc7851..7730e520ff 100644 --- a/hermes_cli/auth_spotify.py +++ b/hermes_cli/auth_spotify.py @@ -1,9 +1,7 @@ """Spotify OAuth (loopback PKCE) login, refresh and runtime credentials. -Split out of ``hermes_cli/auth.py``; every moved name is re-imported there, so -``hermes_cli.auth.`` keeps resolving (and monkeypatching) as before. Origin-internal -helpers are imported lazily inside each function (no import cycle; patches on -``hermes_cli.auth.`` still intercept). +Re-exported from ``hermes_cli/auth.py`` (patch targets unchanged); origin helpers are imported +lazily per function so ``hermes_cli.auth.`` patches still intercept and no cycle forms. """ from __future__ import annotations @@ -26,7 +24,6 @@ from hermes_cli.auth_constants import ( _spotify_err, httpx, ) -# Log-record parity with the origin module (caplog tests pin "hermes_cli.auth"). logger = logging.getLogger("hermes_cli.auth") _CALLBACK_HTML = "

Spotify authorization {}.

You can close this tab." @@ -366,8 +363,7 @@ def _spotify_interactive_setup(redirect_uri_hint: str) -> str: print(f"\nNo Client ID entered. See {SPOTIFY_DOCS_URL} for the full guide.") raise SystemExit("Spotify setup cancelled: empty Client ID.") - # Persist so subsequent `hermes auth spotify` runs skip the wizard. Only persist a non-default - # redirect URI, to avoid pinning users to a value the default might later change to. + # Persist so later runs skip the wizard; only pin a NON-default redirect URI. save_env_value("HERMES_SPOTIFY_CLIENT_ID", raw) if redirect_uri_hint and redirect_uri_hint != DEFAULT_SPOTIFY_REDIRECT_URI: save_env_value("HERMES_SPOTIFY_REDIRECT_URI", redirect_uri_hint) @@ -380,8 +376,7 @@ def login_spotify_command(args) -> None: from hermes_cli.auth import _auth_store_lock, _can_open_graphical_browser, _is_remote_session, _load_auth_store, _print_loopback_ssh_hint, _save_auth_store, _store_provider_state, get_provider_auth_state existing_state = get_provider_auth_state("spotify") or {} - # Interactive wizard: if no client_id is configured anywhere, walk the user through creating - # the Spotify developer app instead of crashing with "HERMES_SPOTIFY_CLIENT_ID is required". + # No client_id anywhere -> wizard instead of "HERMES_SPOTIFY_CLIENT_ID is required". try: client_id = _spotify_client_id(getattr(args, "client_id", None), existing_state) except AuthError as exc: diff --git a/hermes_cli/auth_xai.py b/hermes_cli/auth_xai.py index 03c248b29f..b983fe68cc 100644 --- a/hermes_cli/auth_xai.py +++ b/hermes_cli/auth_xai.py @@ -1,9 +1,7 @@ """xAI Grok OAuth: token store, discovery, refresh, device-code login. -Split out of ``hermes_cli/auth.py``; every moved name is re-imported there, so -``hermes_cli.auth.`` keeps resolving (and monkeypatching) as before. Origin-internal -helpers are imported lazily inside each function (no import cycle; patches on -``hermes_cli.auth.`` still intercept). +Re-exported from ``hermes_cli/auth.py`` (patch targets unchanged); origin helpers are imported +lazily per function so ``hermes_cli.auth.`` patches still intercept and no cycle forms. """ from __future__ import annotations @@ -26,7 +24,6 @@ from utils import env_float if TYPE_CHECKING: # annotation-only; the runtime import would be a cycle from hermes_cli.auth import ProviderConfig -# Log-record parity with the origin module (caplog tests pin "hermes_cli.auth"). logger = logging.getLogger("hermes_cli.auth") _RELOGIN = "Re-authenticate with `hermes model`." @@ -101,20 +98,18 @@ def _read_xai_oauth_tokens(*, _lock: bool = True) -> Dict[str, Any]: def _write_through_xai_oauth_to_global_root(state: Dict[str, Any]) -> None: - """Persist a rotated xAI OAuth ``state`` into the global-root auth.json (best effort). + """Best-effort persist of a rotated xAI grant into the global-root auth.json. - xAI rotates the refresh_token on every refresh, so when a profile session refreshes a grant it - resolved from the root fallback, the rotated chain must land back in root. Only updates - ``providers.xai-oauth`` in root; never touches the profile store (the caller saved that). - Swallows all errors — a failed write-through degrades to root-stale, never breaks the profile save. + xAI rotates refresh_token on every refresh, so a profile that refreshed a root-resolved grant + must write the chain back to root. Touches only root ``providers.xai-oauth``; swallows all + errors (root-stale is better than breaking the profile's own save). """ from hermes_cli.auth import _global_auth_file_path, _persist_provider_state_to_store global_path = _global_auth_file_path() if global_path is None: # classic mode (profile == root); the profile save already hit root return - # Seat belt: under pytest, refuse to write the real user's ~/.hermes/auth.json even when - # HERMES_HOME points at a profile path (mirrors the read-side guard in _load_global_auth_store). - # Uses the unmodified HOME env, not Path.home() which fixtures may monkeypatch. + # Seat belt: under pytest never write the real ~/.hermes/auth.json (mirrors the read-side guard + # in _load_global_auth_store). Uses raw HOME, not Path.home(), which fixtures may monkeypatch. real_home_env = os.environ.get("HOME", "") if os.environ.get("PYTEST_CURRENT_TEST") else "" if real_home_env: real_root = Path(real_home_env) / ".hermes" / "auth.json" @@ -134,21 +129,19 @@ def _save_xai_oauth_tokens( last_refresh: Optional[str] = None, auth_mode: str = "oauth_device_code", set_active: bool = True, ) -> None: - """Persist xAI OAuth tokens into the auth store. + """Persist xAI OAuth tokens; *set_active* also promotes ``xai-oauth`` to ``active_provider``. - *set_active* (default True) also promotes ``xai-oauth`` to ``active_provider`` — right for an - intentional login. Pass False for side-tool credential bootstrap (TTS/setup, tools config, - dashboard token save, token refresh) so inference routing is unchanged. + Pass ``set_active=False`` for side-tool bootstrap (TTS/setup, tools config, dashboard, refresh) + so inference routing is unchanged. """ from hermes_cli.auth import _auth_store_lock, _global_auth_file_path, _load_auth_store, _load_provider_state_with_source, _same_path, _save_auth_store, _store_provider_state, _utc_now_z, _write_through_xai_oauth_to_global_root if last_refresh is None: last_refresh = _utc_now_z() with _auth_store_lock(): auth_store = _load_auth_store() - # A profile without its own xai-oauth block reads the root grant through - # _load_provider_state's fallback. Refreshing that (rotating) grant must write the rotated - # chain back to root, or root keeps a revoked refresh token. Decide by where the grant was - # resolved FROM, not by key presence: _store_provider_state below would create the key. + # A profile lacking its own xai-oauth block reads root's grant via fallback; refreshing it + # must write the rotated chain back to root or root keeps a revoked refresh token. Decide by + # where the grant was resolved FROM (key presence lies: _store_provider_state creates it). state, source_path = _load_provider_state_with_source(auth_store, "xai-oauth") state = state if state is not None else {} state.update(tokens=tokens, last_refresh=last_refresh, auth_mode=auth_mode) @@ -158,8 +151,7 @@ def _save_xai_oauth_tokens( state["redirect_uri"] = redirect_uri global_root = _global_auth_file_path() if source_path is not None and global_root is not None and _same_path(source_path, global_root): - # Resolved from root — write back to root only. Storing on the profile auth_store would - # create a shadowing providers.xai-oauth key that disables write-through next refresh. + # Root-only write-back: a profile copy would shadow root and disable write-through. _write_through_xai_oauth_to_global_root(state) else: _store_provider_state(auth_store, "xai-oauth", state, set_active=set_active) @@ -187,12 +179,10 @@ def _xai_access_token_is_expiring(access_token: str, skew_seconds: int = 0) -> b def _xai_proactive_refresh_skew_seconds(access_token: str) -> int: - """How far before JWT ``exp`` to proactively refresh xAI OAuth tokens. + """Proactive-refresh lead time before JWT ``exp``. - SuperGrok sessions ship multi-hour tokens where the hour-long skew makes sense, but device-code - logins often return ~15-minute JWTs; the full skew would force a refresh on every credential - resolution, burning single-use refresh tokens and racing concurrent callers into - ``invalid_grant`` quarantine. + Device-code logins often return ~15-minute JWTs; the full hour-long skew would refresh on every + resolution, burning single-use refresh tokens and racing callers into ``invalid_grant``. """ max_skew = XAI_ACCESS_TOKEN_REFRESH_SKEW_SECONDS exp = _xai_jwt_exp(access_token) @@ -221,11 +211,10 @@ def _xai_url_problem(url: str) -> tuple[Optional[str], str]: def _xai_validate_oauth_endpoint(url: str, *, field: str) -> str: - """Refuse any OIDC discovery endpoint that isn't HTTPS on the xAI origin. + """Refuse a discovery endpoint that isn't HTTPS on the xAI origin. - The discovery result is cached in auth.json, so a single MITM at login could plant a malicious - ``token_endpoint`` that receives the refresh_token forever. Pinning scheme + host (RFC 8414 §2) - removes that persistence. + Discovery is cached in auth.json, so one MITM at login could plant a ``token_endpoint`` that + receives the refresh_token forever; pinning scheme + host (RFC 8414 §2) removes that. """ problem, host = _xai_url_problem(url) if problem is None: @@ -244,11 +233,9 @@ def _xai_validate_oauth_endpoint(url: str, *, field: str) -> str: def _xai_validate_inference_base_url(value: str, *, fallback: str) -> str: - """Refuse a non-xAI base_url for the OAuth-authenticated inference path. + """Pin the OAuth inference base_url to ``*.x.ai``; warn and use *fallback* on rejection. - Pins the inference origin to ``api.x.ai`` (or any ``*.x.ai`` subdomain). On rejection, fall - back to the default and log a warning rather than raise — a bad env var should not deadlock - authentication, but it must never leak the bearer. Empty input returns ``fallback``. + Warn-not-raise: a bad env var must not deadlock auth, but the bearer must never leak elsewhere. """ candidate = (value or "").strip().rstrip("/") if not candidate: @@ -321,9 +308,8 @@ def refresh_xai_oauth_pure( f"xAI OAuth is missing refresh_token. {_RELOGIN}", "xai_auth_missing_refresh_token", relogin=True, ) endpoint = token_endpoint.strip() or _xai_oauth_discovery(timeout_seconds)["token_endpoint"] - # Re-validate cached endpoints on the refresh hot path: an auth.json written by an older Hermes - # (or hand-edited) may carry a non-xAI token_endpoint that would receive every future - # refresh_token in plaintext if trusted blindly. + # Re-validate cached endpoints: an old/hand-edited auth.json may carry a non-xAI token_endpoint + # that would otherwise receive every future refresh_token. _xai_validate_oauth_endpoint(endpoint, field="token_endpoint") timeout = httpx.Timeout(max(5.0, float(timeout_seconds))) with httpx.Client(timeout=timeout, headers={"Accept": "application/json"}) as client: @@ -334,9 +320,8 @@ def refresh_xai_oauth_pure( if response.status_code != 200: detail = response.text.strip() suffix = f" Response: {detail}" if detail else "" - # 403 from xAI's token endpoint is almost always a tier / entitlement gate (the grant exists - # but the account isn't allowlisted for API access). Re-running `hermes model` won't fix - # that — use a separate code so format_auth_error skips the re-authenticate hint. + # 403 is almost always a tier/entitlement gate; re-login won't fix it, so use a separate + # code and format_auth_error skips the re-authenticate hint. if response.status_code == 403: raise _xai_err( "xAI token refresh failed with HTTP 403." + suffix @@ -365,8 +350,7 @@ def refresh_xai_oauth_pure( def _refresh_xai_oauth_tokens( tokens: Dict[str, Any], *, token_endpoint: str, redirect_uri: str = "", timeout_seconds: float ) -> Dict[str, Any]: - # Re-persist whatever auth_mode is already stored (legacy pre-device-code logins may still - # carry ``oauth_pkce``): the refresh hot path must not relabel how the grant was obtained. + # Keep the stored auth_mode (legacy logins may carry ``oauth_pkce``): refresh must not relabel it. from hermes_cli.auth import _load_auth_store, _load_provider_state, refresh_xai_oauth_pure try: state = _load_provider_state(_load_auth_store(), "xai-oauth") or {} @@ -386,8 +370,7 @@ def _refresh_xai_oauth_tokens( updated_tokens["expires_in"] = refreshed["expires_in"] if refreshed.get("token_type"): updated_tokens["token_type"] = refreshed["token_type"] - # set_active=False: refresh must not flip active_provider — TTS/side tools can refresh xAI - # tokens while chat still routes through another provider. + # set_active=False: side tools (TTS) refresh xAI tokens while chat routes elsewhere. _save_xai_oauth_tokens( updated_tokens, discovery={"token_endpoint": token_endpoint}, redirect_uri=redirect_uri, last_refresh=refreshed["last_refresh"], auth_mode=auth_mode, set_active=False, @@ -396,11 +379,9 @@ def _refresh_xai_oauth_tokens( def _quarantine_xai_oauth_tokens(exc: AuthError) -> None: - """Clear dead xAI tokens from auth.json after a terminal refresh failure. + """Clear dead xAI tokens after a terminal (400/401/403) refresh failure so later sessions fail fast. - Terminal = HTTP 400/401/403 (invalid_grant, token revoked). Subsequent sessions then fail fast - without a network retry. Best-effort: persistence failures are logged and swallowed (caller - re-raises the original error regardless). + Best-effort: persistence failures are logged and swallowed; the caller re-raises regardless. """ from hermes_cli.auth import _last_auth_error_marker, _load_auth_store, _load_provider_state, _save_auth_store, _store_provider_state try: @@ -471,8 +452,7 @@ def resolve_xai_oauth_runtime_credentials( "api_key": _clean(tokens.get("access_token")), "source": "hermes-auth-store", "last_refresh": data.get("last_refresh"), - # Display/telemetry only. Device-code is the only supported xAI OAuth flow, so report it - # unconditionally — auth.json may still carry a legacy ``oauth_pkce`` label. + # Display only; auth.json may still carry a legacy ``oauth_pkce`` label. "auth_mode": "oauth_device_code", } @@ -506,11 +486,9 @@ def _login_xai_oauth(args, pconfig: ProviderConfig, *, force_new_login: bool = F redirect_uri=creds.get("redirect_uri", ""), last_refresh=creds.get("last_refresh"), auth_mode="oauth_device_code", ) - # An explicit interactive re-login means the user wants the xAI credential re-enabled. - # ``hermes auth remove xai-oauth`` leaves a ``device_code`` suppression marker that otherwise - # stops the singleton seed from re-creating the pool entry. Kept OUT of _save_xai_oauth_tokens - # on purpose — that helper is shared with the refresh hot path, which must never mutate - # suppression state. + # Explicit re-login re-enables the credential: clear the ``device_code`` suppression marker left + # by ``hermes auth remove xai-oauth``. Deliberately NOT inside _save_xai_oauth_tokens — the + # refresh hot path shares that helper and must never mutate suppression state. unsuppress_credential_source("xai-oauth", "device_code") config_path = _update_config_for_provider("xai-oauth", creds.get("base_url", DEFAULT_XAI_OAUTH_BASE_URL)) _print_login_success("xai-oauth", config_path, show_auth_state=True) diff --git a/hermes_cli/auth_zai_kimi.py b/hermes_cli/auth_zai_kimi.py index c343cc696d..1985bb44ab 100644 --- a/hermes_cli/auth_zai_kimi.py +++ b/hermes_cli/auth_zai_kimi.py @@ -1,9 +1,7 @@ """Kimi Code and Z.AI endpoint auto-detection, LM Studio base-URL normalization. -Split out of ``hermes_cli/auth.py``; every moved name is re-imported there, so -``hermes_cli.auth.`` keeps resolving (and monkeypatching) as before. Origin-internal -helpers are imported lazily inside each function (no import cycle; patches on -``hermes_cli.auth.`` still intercept). +Re-exported from ``hermes_cli/auth.py`` (patch targets unchanged); origin helpers are imported +lazily per function so ``hermes_cli.auth.`` patches still intercept and no cycle forms. """ from __future__ import annotations @@ -13,13 +11,10 @@ import hashlib from typing import Dict, Optional from hermes_cli.auth_constants import httpx -# Log-record parity with the origin module (caplog tests pin "hermes_cli.auth"). logger = logging.getLogger("hermes_cli.auth") -# Kimi Code (kimi.com/code) issues "sk-kimi-" keys that only work on api.kimi.com/coding; legacy -# platform.moonshot.ai keys work on api.moonshot.ai/v1 (the old default). Intentionally NO /v1 -# suffix: the /coding endpoint speaks Anthropic Messages and the SDK appends "/v1/messages" -# itself — "/coding/v1" would produce "/coding/v1/v1/messages" (a 404). +# "sk-kimi-" keys only work on api.kimi.com/coding; legacy moonshot keys use the old default. +# NO /v1 suffix: the anthropic SDK appends "/v1/messages" itself ("/coding/v1" would 404). KIMI_CODE_BASE_URL = "https://api.kimi.com/coding" @@ -32,10 +27,9 @@ def _resolve_kimi_base_url(api_key: str, default_url: str, env_override: str) -> return default_url -# Z.AI bills general vs coding plans, and global vs China endpoints, separately; a key that works -# on one may return "Insufficient balance" on another, so we probe at setup time and store the -# working endpoint. Each entry lists candidate models to try in order — newer coding-plan accounts -# may only have access to recent models while older ones still use glm-4.7. +# Z.AI bills general/coding plans and global/China endpoints separately ("Insufficient balance" on +# the wrong one), so probe once and cache. Candidate models are tried in order: newer coding-plan +# accounts may only have recent GLM slugs, older ones still glm-4.7. _ZAI_CODING_PROBE_MODELS = ["glm-5.3", "glm-5.3-flash", "glm-5.2", "glm-5.1", "glm-5v-turbo", "glm-4.7"] ZAI_ENDPOINTS = [ # (id, base_url, probe_models, label) @@ -69,8 +63,7 @@ def _probe_single_zai_endpoint(api_key: str, endpoint: tuple, timeout: float) -> def detect_zai_endpoint(api_key: str, timeout: float = 8.0) -> Optional[Dict[str, str]]: """Probe z.ai endpoints in parallel; first working one in ZAI_ENDPOINTS priority order, or None.""" from concurrent.futures import ThreadPoolExecutor, as_completed - # No `with` block: a context manager would join ALL probe threads on exit, defeating the early - # return below. shutdown(wait=False) lets surviving probes drain in the background. + # No `with`: it would join ALL probes on exit, defeating the early return below. pool = ThreadPoolExecutor(max_workers=len(ZAI_ENDPOINTS)) try: futures = {pool.submit(_probe_single_zai_endpoint, api_key, ep, timeout): ep[0] for ep in ZAI_ENDPOINTS} @@ -111,8 +104,7 @@ def _resolve_zai_base_url(api_key: str, default_url: str, env_override: str) -> from hermes_cli.auth import _auth_store_lock, _load_auth_store, _load_provider_state, _save_auth_store, _store_provider_state, detect_zai_endpoint if env_override: return env_override - # No API key → don't probe (N×M HTTPS requests with an empty Bearer, all 401). Hit during - # auxiliary-client auto-detection for users with no Z.AI credentials at all — pure latency. + # No key -> don't probe (N×M 401s); auxiliary-client auto-detection hits this for everyone. if not api_key: return default_url @@ -134,15 +126,13 @@ def _resolve_zai_base_url(api_key: str, default_url: str, env_override: str) -> "model": detected.get("model", ""), "label": detected.get("label", ""), "key_hash": key_hash, } - # Persist failure (disk full, permissions, lock timeout) must not break resolution — detection - # already succeeded; worst case the next start re-probes. + # Persist failure must not break resolution; worst case the next start re-probes. try: with _auth_store_lock(): auth_store = _load_auth_store() # reload under lock to avoid overwriting concurrent changes state_under_lock = _load_provider_state(auth_store, "zai") or {} state_under_lock["detected_endpoint"] = detected_endpoint - # set_active=False: this runs from credential-pool env seeding for ANY user with a Z.AI - # key in env, and caching a probe result must not flip their active provider. + # set_active=False: runs from credential-pool env seeding; must not flip active provider. _store_provider_state(auth_store, "zai", state_under_lock, set_active=False) _save_auth_store(auth_store) except Exception as exc: From f73578de3a24f0da17be3e9a01f0cd77038b4472 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 22:34:28 -0700 Subject: [PATCH 16/16] =?UTF-8?q?refactor(hermes=5Fcli):=20approvals=5Fsug?= =?UTF-8?q?gest=20=E2=80=94=20group=20unsafe-class=20patterns=20by=20theme?= =?UTF-8?q?=20(same=20set,=20boolean=20search)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/approvals_suggest.py | 46 ++++++++------------------------- 1 file changed, 11 insertions(+), 35 deletions(-) diff --git a/hermes_cli/approvals_suggest.py b/hermes_cli/approvals_suggest.py index 3965414210..29acffa183 100644 --- a/hermes_cli/approvals_suggest.py +++ b/hermes_cli/approvals_suggest.py @@ -28,41 +28,17 @@ from typing import Iterable, Iterator, Optional # descriptions). Deliberately conservative: a benign class accidentally excluded costs the user # one manual config edit; a destructive class accidentally proposed costs them data. _UNSAFE_CLASS_PATTERNS = [ - r"delete", # recursive delete, find -delete, branch force delete, ... - r"\brm\b", # xargs with rm, find -exec rm - r"destro", # git reset --hard (destroys ...), destructive - r"wipe", - r"format", # format filesystem - r"\bdisk\b", - r"block device", - r"fork bomb", - r"kill (?:all )?process", # force/regex/all process kills - r"kill all", - r"self-termination", - r"\bsudo\b", - r"privilege", - r"credential", - r"\bssh\b", - r"shell.rc", - r"system config", - r"system file", - r"\bsql\b", # SQL DROP / TRUNCATE / DELETE without WHERE - r"\bchown\b", - r"\bchmod\b", - r"writable", # world/other-writable permissions - r"overwrite", - r"in-place edit", - r"pipe", # pipe remote/decoded content to shell - r"obfuscation", - r"remote content", - r"remote script", - r"heredoc", - r"encoded", # PowerShell encoded command execution - r"command substitution", - r"process substitution", - r"\bdd\b", - r"shutdown", - r"reboot", + # deletion / destruction: recursive delete, find -delete, xargs rm, git reset --hard, mkfs, dd + r"delete", r"\brm\b", r"destro", r"wipe", r"format", r"\bdisk\b", r"block device", r"\bdd\b", + # process / host: fork bomb, force/regex/all kills, self-termination, shutdown + r"fork bomb", r"kill (?:all )?process", r"kill all", r"self-termination", r"shutdown", r"reboot", + # privilege / credentials / system files + r"\bsudo\b", r"privilege", r"credential", r"\bssh\b", r"shell.rc", r"system config", r"system file", + # data & permissions: SQL DROP/TRUNCATE/DELETE-without-WHERE, chown/chmod, world-writable, overwrite + r"\bsql\b", r"\bchown\b", r"\bchmod\b", r"writable", r"overwrite", r"in-place edit", + # remote/obfuscated execution: pipe-to-shell, encoded PowerShell, heredoc, substitutions + r"pipe", r"obfuscation", r"remote content", r"remote script", r"heredoc", r"encoded", + r"command substitution", r"process substitution", r"hardline", ] _UNSAFE_CLASS_RE = re.compile("|".join(_UNSAFE_CLASS_PATTERNS), re.IGNORECASE)