diff --git a/agent/credential_pool.py b/agent/credential_pool.py index 4beb3492b083a..e7e2ab800f7c6 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -188,6 +188,12 @@ class PooledCredential: agent_key: Optional[str] = None agent_key_expires_at: Optional[str] = None request_count: int = 0 + # Optional user-assigned name (``hermes auth add --name ``) + # used for manual credential selection via ``config.default_auth``. Unlike + # ``label`` (a display label that singleton seeding may rewrite), ``name`` + # is a stable selection key that the user controls. Entries without a + # name keep the legacy auto-rotate behavior. + name: Optional[str] = None extra: Dict[str, Any] = None # type: ignore[assignment] def __post_init__(self): @@ -486,6 +492,30 @@ def get_pool_strategy(provider: str) -> str: return STRATEGY_FILL_FIRST +def get_default_auth_name(provider: str) -> Optional[str]: + """Return the user-selected credential name for a provider, if any. + + Reads ``config.yaml``'s ``default_auth`` map (``default_auth: + {: }``). When set, the pool prefers the + credential whose ``name`` matches — manual selection instead of passive + auto-rotation (#76937). Selection falls back to the normal strategy when + the named credential is missing or currently exhausted. A missing or + malformed config returns None, preserving the legacy auto-rotate + behavior for every existing pool. + """ + config = _load_config_safe() + if config is None: + return None + default_auth = config.get("default_auth") + if not isinstance(default_auth, dict): + return None + name = default_auth.get(provider) + if not isinstance(name, str): + return None + name = name.strip() + return name or None + + def credential_pool_matches_provider( pool_or_provider: Any, provider: Optional[str], @@ -588,6 +618,12 @@ def __init__(self, provider: str, entries: List[PooledCredential]): self._entries = sorted(entries, key=lambda entry: entry.priority) self._current_id: Optional[str] = None self._strategy = get_pool_strategy(provider) + # Optional manual-selection name from config ``default_auth`` + # (#76937). When set and a matching entry is available, selection is + # pinned to that credential instead of passive auto-rotation. + # Per-session override via HERMES_AUTH_NAME env var takes precedence. + session_auth = os.getenv("HERMES_AUTH_NAME", "").strip() + self._default_auth = session_auth or get_default_auth_name(provider) self._lock = threading.Lock() self._active_leases: Dict[str, int] = {} self._max_concurrent = DEFAULT_MAX_CONCURRENT_PER_CREDENTIAL @@ -1800,6 +1836,35 @@ def _log_no_available_entries(self) -> None: self._last_no_entries_log_at = now logger.info("credential pool: no available entries (all exhausted or empty)") + def _narrow_to_default_auth_unlocked( + self, available: List[PooledCredential] + ) -> List[PooledCredential]: + """Pin selection to the ``default_auth`` named credential when possible. + + #76937 manual selection: when the user configured a preferred + credential name for this provider, selection is restricted to entries + with that name while one of them is available. If the named entry is + missing or currently exhausted it drops out of ``available`` and the + full list is returned, so the pool falls back to the legacy + auto-rotate behavior instead of failing the request. + """ + if not self._default_auth: + return available + named = [ + entry + for entry in available + if (entry.name or "").strip() == self._default_auth + ] + if named: + return named + logger.info( + "credential pool: default_auth credential %r for %s unavailable " + "(missing or exhausted) — falling back to auto-rotation", + self._default_auth, + self.provider, + ) + return available + def _select_unlocked(self, *, refresh: bool = True) -> Optional[PooledCredential]: available = self._available_entries(clear_expired=True, refresh=refresh) if not available: @@ -1807,6 +1872,11 @@ def _select_unlocked(self, *, refresh: bool = True) -> Optional[PooledCredential self._log_no_available_entries() return None + # Manual selection (#76937): pin to the configured default_auth + # credential while it is available; fall back to auto-rotate below + # when it is missing or exhausted. + available = self._narrow_to_default_auth_unlocked(available) + # A successful selection means the pool recovered; re-arm the throttle # so a later re-exhaustion logs immediately rather than being silenced # by a window opened during the previous empty stretch. @@ -1846,6 +1916,7 @@ def peek(self) -> Optional[PooledCredential]: if current is not None: return current available = self._available_entries() + available = self._narrow_to_default_auth_unlocked(available) return available[0] if available else None def mark_exhausted_and_rotate( @@ -1996,6 +2067,12 @@ def acquire_lease(self, credential_id: Optional[str] = None) -> Optional[str]: if not available: return None + # Manual selection (#76937): pin to the configured default_auth + # credential while it is available; fall back to auto-rotate. + available = self._narrow_to_default_auth_unlocked(available) + if not available: + return None + below_cap = [ entry for entry in available if self._active_leases.get(entry.id, 0) < self._max_concurrent @@ -2379,6 +2456,10 @@ def _env_val(key: str) -> str: "agent_key_obtained_at": state.get("agent_key_obtained_at"), "tls": state.get("tls") if isinstance(state.get("tls"), dict) else None, "label": seeded_label, + # Manual-selection name (#76937) embedded by + # persist_nous_credentials(name=...) — None when unset, + # which _upsert_entry ignores. + "name": state.get("name"), }, ) diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 82eb5ee7db5ee..59da489729347 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -6034,6 +6034,7 @@ def persist_nous_credentials( creds: Dict[str, Any], *, label: Optional[str] = None, + name: Optional[str] = None, ): """Persist Nous OAuth credentials as the singleton provider state and ensure the credential pool is in sync. @@ -6070,6 +6071,11 @@ def persist_nous_credentials( state = dict(creds) if label and str(label).strip(): state["label"] = str(label).strip() + # Optional manual-selection name (#76937) — carried through the + # singleton state so ``_seed_from_singletons`` re-applies it on every + # subsequent ``load_pool(\"nous\")``. + if name and str(name).strip(): + state["name"] = str(name).strip() with _auth_store_lock(): auth_store = _load_auth_store() diff --git a/hermes_cli/auth_commands.py b/hermes_cli/auth_commands.py index a5b3e7210b8ed..ba1d8fc67b318 100644 --- a/hermes_cli/auth_commands.py +++ b/hermes_cli/auth_commands.py @@ -22,6 +22,7 @@ PooledCredential, _exhausted_until, _normalize_custom_pool_name, + get_default_auth_name, get_pool_strategy, label_from_token, list_custom_pool_providers, @@ -109,6 +110,19 @@ def _api_key_default_label(count: int) -> str: return f"api-key-{count}" +def _credential_name(args) -> Optional[str]: + """Return a user-supplied credential name (``--name``), stripped, or None. + + The name is the manual-selection key for #76937: pairing it with + ``config.default_auth`` pins the provider's pool to this credential. + """ + name = getattr(args, "name", None) + if not isinstance(name, str): + return None + name = name.strip() + return name or None + + def _display_source(source: str) -> str: return source.split(":", 1)[1] if source.startswith("manual:") else source @@ -216,6 +230,7 @@ def auth_add_command(args) -> None: source=SOURCE_MANUAL, access_token=token, base_url=_provider_base_url(provider), + name=_credential_name(args), ) pool.add_entry(entry) print(f'Added {provider} credential #{len(pool.entries())}: "{label}"') @@ -242,6 +257,7 @@ def auth_add_command(args) -> None: refresh_token=creds.get("refresh_token"), expires_at_ms=creds.get("expires_at_ms"), base_url=_provider_base_url(provider), + name=_credential_name(args), ) pool.add_entry(entry) print(f'Added {provider} OAuth credential #{len(pool.entries())}: "{entry.label}"') @@ -276,7 +292,11 @@ def auth_add_command(args) -> None: ) if rehydrated is not None: custom_label = (getattr(args, "label", None) or "").strip() or None - entry = auth_mod.persist_nous_credentials(rehydrated, label=custom_label) + entry = auth_mod.persist_nous_credentials( + rehydrated, + label=custom_label, + name=_credential_name(args), + ) shown_label = entry.label if entry is not None else label_from_token( rehydrated.get("access_token", ""), _oauth_default_label(provider, 1), ) @@ -300,7 +320,11 @@ def auth_add_command(args) -> None: # helper embeds this into providers.nous so that label_from_token # doesn't overwrite it on every subsequent load_pool("nous"). custom_label = (getattr(args, "label", None) or "").strip() or None - entry = auth_mod.persist_nous_credentials(creds, label=custom_label) + entry = auth_mod.persist_nous_credentials( + creds, + label=custom_label, + name=_credential_name(args), + ) shown_label = entry.label if entry is not None else label_from_token( creds.get("access_token", ""), _oauth_default_label(provider, 1), ) @@ -334,6 +358,7 @@ def auth_add_command(args) -> None: refresh_token=creds["tokens"].get("refresh_token"), base_url=creds.get("base_url"), last_refresh=creds.get("last_refresh"), + name=_credential_name(args), ) first_credential = not pool.entries() pool.add_entry(entry) @@ -375,6 +400,7 @@ def auth_add_command(args) -> None: refresh_token=creds["tokens"].get("refresh_token"), base_url=creds.get("base_url") or auth_mod.DEFAULT_XAI_OAUTH_BASE_URL, last_refresh=creds.get("last_refresh"), + name=_credential_name(args), ) first_credential = not pool.entries() pool.add_entry(entry) @@ -402,6 +428,7 @@ def auth_add_command(args) -> None: source=f"{SOURCE_MANUAL}:qwen_cli", access_token=creds["api_key"], base_url=creds.get("base_url"), + name=_credential_name(args), ) pool.add_entry(entry) print(f'Added {provider} OAuth credential #{len(pool.entries())}: "{entry.label}"') @@ -426,6 +453,7 @@ def auth_add_command(args) -> None: access_token=creds["access_token"], refresh_token=creds.get("refresh_token"), base_url=creds.get("inference_base_url"), + name=_credential_name(args), ) pool.add_entry(entry) print(f'Added {provider} OAuth credential #{len(pool.entries())}: "{entry.label}"') @@ -450,14 +478,21 @@ def auth_list_command(args) -> None: if not entries: continue current = pool.peek() - print(f"{provider} ({len(entries)} credentials):") + default_auth = get_default_auth_name(provider) + header = f"{provider} ({len(entries)} credentials):" + if default_auth: + header += f" [default_auth: {default_auth}]" + print(header) for idx, entry in enumerate(entries, start=1): marker = " " if current is not None and entry.id == current.id: marker = "← " status = _format_exhausted_status(entry) source = _display_source(entry.source) - print(f" #{idx} {entry.label:<20} {entry.auth_type:<7} {source}{status} {marker}".rstrip()) + name_tag = f" [name:{entry.name}]" if (entry.name or "").strip() else "" + print( + f" #{idx} {entry.label:<20} {entry.auth_type:<7} {source}{name_tag}{status} {marker}".rstrip() + ) print() @@ -698,8 +733,18 @@ def _interactive_add() -> None: if typed_label: label = typed_label + name = None + try: + typed_name = input( + "Credential name for manual selection, e.g. 'daily' (optional): " + ).strip() + except (EOFError, KeyboardInterrupt): + return + if typed_name: + name = typed_name + auth_add_command(SimpleNamespace( - provider=provider, auth_type=auth_type, label=label, api_key=None, + provider=provider, auth_type=auth_type, label=label, name=name, api_key=None, portal_url=None, inference_url=None, client_id=None, scope=None, no_browser=False, timeout=None, insecure=False, ca_bundle=None, )) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index ff4629dacad78..237f83eef3862 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -4654,6 +4654,7 @@ def _default_value_for_key(dotted_key: str): _OPEN_DICT_TOP_LEVEL_KEYS = frozenset({ "providers", "credential_pool_strategies", + "default_auth", "mcp_servers", "hooks", "quick_commands", diff --git a/hermes_cli/config_defaults.py b/hermes_cli/config_defaults.py index b070819017413..8283338d24bda 100644 --- a/hermes_cli/config_defaults.py +++ b/hermes_cli/config_defaults.py @@ -9,6 +9,11 @@ "providers": {}, "fallback_providers": [], "credential_pool_strategies": {}, + # Manual credential selection (#76937): map provider -> credential name + # (the ``--name`` given to ``hermes auth add``). The provider's pool is + # pinned to that credential while it is available, falling back to + # auto-rotation when it is missing or exhausted. + "default_auth": {}, "toolsets": ["hermes-cli"], # SQLite journal mode used by every Hermes database opener. WAL is the # normal default; set DELETE for weak-fsync/shared filesystems where WAL is diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 98874824c196c..6a1a909454260 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -11118,6 +11118,10 @@ def cmd_skills(args): from hermes_cli.skills_config import skills_command as skills_config_command skills_config_command(args) + elif getattr(args, "skills_action", None) == "group": + from hermes_cli.skills_groups import group_command + + group_command(args) else: from hermes_cli.skills_hub import skills_command diff --git a/hermes_cli/skills_groups.py b/hermes_cli/skills_groups.py new file mode 100644 index 0000000000000..0b1c2e6a214eb --- /dev/null +++ b/hermes_cli/skills_groups.py @@ -0,0 +1,265 @@ +"""Skill groups — organize installed skills into named groups. + +Groups keep large skill installs navigable. They are stored in +``config.yaml`` under ``skills.groups`` as a mapping of group name to a +list of skill names:: + + skills: + disabled: [skill-a] + groups: + security: [web-pentest, godmode] + writing: [humanizer] + +Groups are purely organizational: they do not change how skills load or +run. They power the ``hermes skills group ...`` subcommands and the +``--group`` filter on ``hermes skills list``. +""" + +from __future__ import annotations + +import json +from typing import Any, Dict, List, Optional, Set + +from rich.console import Console +from rich.table import Table + +from hermes_cli.config import load_config, save_config + +_console = Console() + + +def get_skill_groups(config: Optional[Dict[str, Any]] = None) -> Dict[str, List[str]]: + """Read ``skills.groups`` from config and normalize it. + + Args: + config: Optional already-loaded config dict (test hook). When + omitted, the active profile's config is loaded — profile + switches (``-p``) swap ``HERMES_HOME`` at process start, so no + explicit profile flag is needed here, mirroring + ``get_disabled_skill_names``. + + Returns a dict of group name -> sorted list of unique skill names. + Tolerates missing/null/malformed sections like ``get_disabled_skills`` + in ``hermes_cli/skills_config.py``. + """ + if config is None: + config = load_config() + skills_cfg = config.get("skills") + if not isinstance(skills_cfg, dict): + return {} + raw = skills_cfg.get("groups") + if not isinstance(raw, dict): + return {} + groups: Dict[str, List[str]] = {} + for name, members in raw.items(): + if not isinstance(name, str) or not name.strip(): + continue + normalized = _normalize_members(members) + if normalized: + groups[name] = normalized + return groups + + +def save_skill_groups(config: Dict[str, Any], groups: Dict[str, List[str]]) -> None: + """Persist ``skills.groups`` into *config* and write it to disk.""" + config.setdefault("skills", {}) + config["skills"]["groups"] = { + name: sorted(set(members)) for name, members in groups.items() + } + save_config(config) + + +def add_skills_to_group( + config: Dict[str, Any], group: str, skills: List[str] +) -> Dict[str, Any]: + """Add skill names to *group*, creating it when missing. + + Returns a result dict with counts so the CLI can report precisely: + ``{"created": bool, "added": [...], "duplicates": [...], "unknown": [...]}``. + """ + groups = get_skill_groups(config) + created = group not in groups + members = set(groups.get(group, [])) + added: List[str] = [] + duplicates: List[str] = [] + for skill in skills: + (duplicates if skill in members else added).append(skill) + members.add(skill) + groups[group] = sorted(members) + save_skill_groups(config, groups) + return { + "created": created, + "added": added, + "duplicates": duplicates, + "unknown": _unknown_skill_names(added), + } + + +def remove_skills_from_group( + config: Dict[str, Any], + group: str, + skills: Optional[List[str]] = None, +) -> Dict[str, Any]: + """Remove skill names from *group*. + + With no *skills*, the whole group is deleted. A group that ends up + empty is removed too. Returns ``{"removed": [...], "group_deleted": + bool, "missing": [...]}``. + """ + groups = get_skill_groups(config) + if group not in groups: + return {"removed": [], "group_deleted": False, "missing": list(skills or [])} + members = set(groups[group]) + if not skills: + del groups[group] + save_skill_groups(config, groups) + return {"removed": sorted(members), "group_deleted": True, "missing": []} + removed = [s for s in skills if s in members] + missing = [s for s in skills if s not in members] + members.difference_update(removed) + if members: + groups[group] = sorted(members) + else: + del groups[group] + save_skill_groups(config, groups) + return {"removed": removed, "group_deleted": not members, "missing": missing} + + +def _normalize_members(members) -> List[str]: + """Normalize a raw group value (list, scalar, or garbage) to a sorted + list of unique, non-empty strings.""" + if members is None: + return [] + if isinstance(members, str): + members = [members] + if not isinstance(members, (list, tuple, set)): + return [] + seen: Set[str] = set() + out: List[str] = [] + for member in members: + name = str(member).strip() if member is not None else "" + if name and name not in seen: + seen.add(name) + out.append(name) + return sorted(out) + + +def _validate_group_name(name: str) -> Optional[str]: + """Return an error message if *name* is not usable as a group name.""" + if not name or not name.strip(): + return "Group name cannot be empty." + if any(ch.isspace() for ch in name): + return f"Group name {name!r} must not contain whitespace." + if name.startswith("-"): + return f"Group name {name!r} must not start with '-' (looks like a flag)." + return None + + +def _installed_skill_names() -> Set[str]: + """Best-effort set of installed skill names (empty on discovery failure).""" + try: + from tools.skills_tool import _find_all_skills + + return { + skill.get("name") + for skill in _find_all_skills(skip_disabled=True) + if skill.get("name") + } + except Exception: + return set() + + +def _unknown_skill_names(names: List[str]) -> List[str]: + """Names in *names* that are not currently installed. + + Returns [] when skill discovery fails (config-only associations are + still allowed — the skill may be installed later). + """ + installed = _installed_skill_names() + if not installed: + return [] + return [name for name in names if name not in installed] + + +def group_command(args) -> None: + """Router for ``hermes skills group `` — called from main.py.""" + action = getattr(args, "group_action", None) + if action in ("list", "ls"): + _cmd_group_list(as_json=getattr(args, "json", False)) + elif action == "add": + _cmd_group_add(args.group, args.skills) + elif action in ("remove", "rm"): + _cmd_group_remove(args.group, getattr(args, "skills", None) or []) + else: + _console.print("Usage: hermes skills group [list|add|remove]\n") + + +def _cmd_group_list(*, as_json: bool = False) -> None: + c = _console + groups = get_skill_groups() + if not groups: + c.print( + "[dim]No skill groups configured. Create one with:[/] " + "hermes skills group add [skill ...]\n" + ) + return + if as_json: + c.print(json.dumps(groups, indent=2)) + return + table = Table(title="Skill Groups") + table.add_column("Group", style="bold cyan") + table.add_column("Skills", style="dim") + for name in sorted(groups): + table.add_row(name, ", ".join(groups[name])) + c.print(table) + total = sum(len(members) for members in groups.values()) + c.print(f"[dim]{len(groups)} group(s), {total} skill assignment(s)[/]\n") + + +def _cmd_group_add(group: str, skills: List[str]) -> None: + c = _console + error = _validate_group_name(group) + if error: + c.print(f"[bold red]Error:[/] {error}\n") + return + if not skills: + c.print( + "[bold red]Error:[/] At least one skill name is required. " + "Usage: hermes skills group add [skill ...]\n" + ) + return + config = load_config() + result = add_skills_to_group(config, group, skills) + verb = "Created group" if result["created"] else "Updated group" + c.print(f"[bold green]{verb}:[/] {group}") + if result["added"]: + c.print(f"[dim]Added: {', '.join(result['added'])}[/]") + if result["duplicates"]: + c.print(f"[dim]Already present: {', '.join(result['duplicates'])}[/]") + if result["unknown"]: + c.print( + f"[yellow]Not installed (added anyway): {', '.join(result['unknown'])}[/]" + ) + c.print() + + +def _cmd_group_remove(group: str, skills: List[str]) -> None: + c = _console + config = load_config() + result = remove_skills_from_group(config, group, skills or None) + if result["group_deleted"]: + c.print(f"[bold green]Deleted group:[/] {group}\n") + return + if result["removed"]: + c.print( + f"[bold green]Removed from {group}:[/] {', '.join(result['removed'])}" + ) + if result["missing"]: + c.print(f"[dim]Not in group: {', '.join(result['missing'])}[/]") + if not result["removed"]: + current = get_skill_groups(config).get(group, []) + c.print( + f"[yellow]No skills removed. Group '{group}' currently has:[/] " + f"{', '.join(current) if current else '(none)'}" + ) + c.print() diff --git a/hermes_cli/skills_hub.py b/hermes_cli/skills_hub.py index dd0aae4686828..3f05ef3d022bf 100644 --- a/hermes_cli/skills_hub.py +++ b/hermes_cli/skills_hub.py @@ -932,12 +932,16 @@ def print(self, *a, **k): def do_list(source_filter: str = "all", enabled_only: bool = False, + group_filter: str = "", console: Optional[Console] = None) -> None: """List installed skills, distinguishing hub, builtin, and local skills. Args: source_filter: ``all`` | ``hub`` | ``builtin`` | ``local``. enabled_only: If True, hide disabled skills from the output. + group_filter: If non-empty, only show skills that belong to this + group (see ``skills.groups`` in config.yaml). Unknown groups + print an error listing the configured groups. Enabled/disabled state is resolved against the currently active profile's config — ``hermes -p skills list`` reads that profile's @@ -948,6 +952,7 @@ def do_list(source_filter: str = "all", from tools.skills_sync import _read_manifest from tools.skills_tool import _find_all_skills from agent.skill_utils import get_disabled_skill_names + from hermes_cli.skills_groups import get_skill_groups c = console or _console ensure_hub_dirs() @@ -959,9 +964,27 @@ def do_list(source_filter: str = "all", all_skills = _find_all_skills(skip_disabled=True) disabled_names = get_disabled_skill_names() + group_members: Optional[set] = None + if group_filter: + groups = get_skill_groups() + group_members = set(groups.get(group_filter, [])) + if group_filter not in groups: + c.print(f"[bold red]Error:[/] Unknown group '{group_filter}'.") + if groups: + c.print(f"[dim]Available groups: {', '.join(sorted(groups))}[/]") + else: + c.print( + "[dim]No groups configured. Create one with: " + "hermes skills group add [skill ...][/]" + ) + c.print() + return + title = "Installed Skills" if enabled_only: title += " (enabled only)" + if group_filter: + title += f" — group '{group_filter}'" table = Table(title=title) table.add_column("Name", style="bold cyan") @@ -1001,6 +1024,9 @@ def do_list(source_filter: str = "all", if enabled_only and not is_enabled: continue + if group_members is not None and name not in group_members: + continue + if source_type == "hub": hub_count += 1 elif source_type == "builtin": @@ -1741,6 +1767,7 @@ def skills_command(args) -> None: do_list( source_filter=args.source, enabled_only=getattr(args, "enabled_only", False), + group_filter=getattr(args, "group", "") or "", ) elif action == "check": do_check(name=getattr(args, "name", None)) @@ -1788,7 +1815,7 @@ def skills_command(args) -> None: return do_tap(tap_action, repo=repo) else: - _console.print("Usage: hermes skills [browse|search|install|inspect|list|list-modified|diff|check|update|audit|uninstall|reset|opt-out|opt-in|publish|snapshot|tap]\n") + _console.print("Usage: hermes skills [browse|search|install|inspect|list|group|list-modified|diff|check|update|audit|uninstall|reset|opt-out|opt-in|publish|snapshot|tap]\n") _console.print("Run 'hermes skills --help' for details.\n") @@ -1918,11 +1945,41 @@ def handle_skills_slash(cmd: str, console: Optional[Console] = None) -> None: elif action == "list": source_filter = "all" enabled_only = "--enabled-only" in args or "--enabled" in args + group_filter = "" if "--source" in args: idx = args.index("--source") if idx + 1 < len(args): source_filter = args[idx + 1] - do_list(source_filter=source_filter, enabled_only=enabled_only, console=c) + if "--group" in args: + from hermes_cli.skills_groups import get_skill_groups # noqa: E402 + idx = args.index("--group") + if idx + 1 < len(args): + raw_group = args[idx + 1] + if raw_group.startswith("--"): + c.print( + f"[bold red]Error:[/] Missing value for --group " + f"(got '{raw_group}', which looks like a flag)." + ) + groups = get_skill_groups() + if groups: + c.print(f"[dim]Available groups: {', '.join(sorted(groups))}[/]") + else: + c.print( + "[dim]No groups configured. Create one with: " + "hermes skills group add [skill ...][/]" + ) + c.print() + return + group_filter = raw_group + else: + c.print("[bold red]Error:[/] --group requires a group name.") + groups = get_skill_groups() + if groups: + c.print(f"[dim]Available groups: {', '.join(sorted(groups))}[/]") + c.print() + return + do_list(source_filter=source_filter, enabled_only=enabled_only, + group_filter=group_filter, console=c) elif action == "check": name = args[0] if args else None diff --git a/hermes_cli/subcommands/auth.py b/hermes_cli/subcommands/auth.py index e81fcea8c100d..654a4616735b9 100644 --- a/hermes_cli/subcommands/auth.py +++ b/hermes_cli/subcommands/auth.py @@ -28,6 +28,11 @@ def build_auth_parser(subparsers, *, cmd_auth: Callable) -> None: help="Credential type to add", ) auth_add.add_argument("--label", help="Optional display label") + auth_add.add_argument( + "--name", + help="Optional credential name for manual selection " + "(set config default_auth. to this name to pin the pool to it)", + ) auth_add.add_argument( "--api-key", help="API key value (otherwise prompted securely)" ) diff --git a/hermes_cli/subcommands/skills.py b/hermes_cli/subcommands/skills.py index 4eb68a01b1c7a..780d339414b80 100644 --- a/hermes_cli/subcommands/skills.py +++ b/hermes_cli/subcommands/skills.py @@ -123,6 +123,12 @@ def build_skills_parser(subparsers, *, cmd_skills: Callable) -> None: help="Hide disabled skills. Use with -p to see exactly " "which skills will load for that profile.", ) + skills_list.add_argument( + "--group", + default="", + help="Only show skills that belong to this group " + "(see `hermes skills group list`)", + ) skills_check = skills_subparsers.add_parser( "check", help="Check installed hub skills for updates" @@ -307,6 +313,49 @@ def build_skills_parser(subparsers, *, cmd_skills: Callable) -> None: tap_rm = tap_subparsers.add_parser("remove", help="Remove a tap") tap_rm.add_argument("name", help="Tap name to remove") + skills_group = skills_subparsers.add_parser( + "group", + help="Organize skills into named groups", + description=( + "Create named groups and associate skills with them. Groups are " + "stored in config.yaml under skills.groups (group name -> list of " + "skill names) and let you filter `hermes skills list --group `." + ), + ) + group_subparsers = skills_group.add_subparsers(dest="group_action") + + group_list = group_subparsers.add_parser( + "list", aliases=["ls"], help="List skill groups and their members" + ) + group_list.add_argument( + "--json", action="store_true", help="Output the groups as JSON" + ) + + group_add = group_subparsers.add_parser( + "add", + help="Add skills to a group (creates the group if needed)", + ) + group_add.add_argument("group", help="Group name (e.g. security)") + group_add.add_argument( + "skills", + nargs="+", + metavar="skill", + help="Skill name(s) to add to the group", + ) + + group_remove = group_subparsers.add_parser( + "remove", + aliases=["rm"], + help="Remove skills from a group, or delete the whole group", + ) + group_remove.add_argument("group", help="Group name") + group_remove.add_argument( + "skills", + nargs="*", + metavar="skill", + help="Skill name(s) to remove. Omit to delete the whole group.", + ) + # config sub-action: interactive enable/disable skills_subparsers.add_parser( "config", diff --git a/tests/agent/test_credential_pool_named_selection.py b/tests/agent/test_credential_pool_named_selection.py new file mode 100644 index 0000000000000..67422f5e6a8a7 --- /dev/null +++ b/tests/agent/test_credential_pool_named_selection.py @@ -0,0 +1,276 @@ +"""Tests for named credential selection in the pool (#76937). + +Covers the two halves of the feature: +- named credentials: an optional ``name`` on a ``PooledCredential`` that + round-trips through the on-disk pool payload (``hermes auth add --name``); +- manual selection: when ``config.default_auth.`` names a + credential, the pool pins selection to it while it is available, and falls + back to the legacy auto-rotate behavior when it is missing or exhausted. +""" + +from __future__ import annotations + +import json +import time + +import pytest + + +def _write_auth_store(tmp_path, payload: dict) -> None: + hermes_home = tmp_path / "hermes" + hermes_home.mkdir(parents=True, exist_ok=True) + (hermes_home / "auth.json").write_text(json.dumps(payload, indent=2)) + + +def _make_pool(entries, *, default_auth=None, strategy="fill_first", monkeypatch=None): + """Build a CredentialPool with controlled strategy and default_auth.""" + from agent import credential_pool as cp + + if monkeypatch is not None: + monkeypatch.setattr(cp, "get_pool_strategy", lambda _p: strategy) + monkeypatch.setattr(cp, "get_default_auth_name", lambda _p: default_auth) + return cp.CredentialPool("openrouter", entries) + + +def _entry(entry_id: str, token: str, *, name=None, priority=0, exhausted=False): + from agent.credential_pool import PooledCredential, STATUS_EXHAUSTED + + kwargs = {} + if name is not None: + kwargs["name"] = name + if exhausted: + kwargs.update( + last_status=STATUS_EXHAUSTED, + last_status_at=time.time(), + last_error_code=429, + last_error_reason="rate_limit", + last_error_reset_at=time.time() + 3600, + ) + return PooledCredential( + provider="openrouter", + id=entry_id, + label=f"label-{entry_id}", + auth_type="api_key", + priority=priority, + source="manual", + access_token=token, + **kwargs, + ) + + +# ── named credential storage ───────────────────────────────────────────── + + +def test_name_round_trips_through_dict_serialization(): + """The optional ``name`` survives to_dict/from_dict (pool persistence).""" + from agent.credential_pool import PooledCredential + + entry = PooledCredential( + provider="openrouter", + id="abc123", + label="work key", + auth_type="api_key", + priority=0, + source="manual", + access_token="sk-or-1", + name="daily", + ) + payload = entry.to_dict() + assert payload["name"] == "daily" + + rehydrated = PooledCredential.from_dict("openrouter", payload) + assert rehydrated.name == "daily" + assert rehydrated.access_token == "sk-or-1" + + # Unnamed entries keep the legacy payload shape (no ``name`` key). + unnamed_payload = PooledCredential( + provider="openrouter", + id="def456", + label="legacy", + auth_type="api_key", + priority=0, + source="manual", + access_token="sk-or-2", + ).to_dict() + assert "name" not in unnamed_payload + + +def test_get_default_auth_name_reads_config(monkeypatch): + from agent import credential_pool as cp + + monkeypatch.setattr(cp, "_load_config_safe", lambda: {"default_auth": {"openrouter": "daily"}}) + assert cp.get_default_auth_name("openrouter") == "daily" + + # Whitespace-collapsed values are honored; empty values are None. + monkeypatch.setattr(cp, "_load_config_safe", lambda: {"default_auth": {"openrouter": " daily "}}) + assert cp.get_default_auth_name("openrouter") == "daily" + monkeypatch.setattr(cp, "_load_config_safe", lambda: {"default_auth": {"openrouter": " "}}) + assert cp.get_default_auth_name("openrouter") is None + + # Missing config, non-dict map, unknown provider, non-str value → None. + monkeypatch.setattr(cp, "_load_config_safe", lambda: None) + assert cp.get_default_auth_name("openrouter") is None + monkeypatch.setattr(cp, "_load_config_safe", lambda: {}) + assert cp.get_default_auth_name("openrouter") is None + monkeypatch.setattr(cp, "_load_config_safe", lambda: {"default_auth": {"anthropic": "x"}}) + assert cp.get_default_auth_name("openrouter") is None + monkeypatch.setattr(cp, "_load_config_safe", lambda: {"default_auth": {"openrouter": 42}}) + assert cp.get_default_auth_name("openrouter") is None + + +# ── manual selection in the pool ────────────────────────────────────────── + + +def test_select_prefers_named_credential(monkeypatch): + """With default_auth set, selection is pinned to the named entry even when + it is not the first entry (fill_first would otherwise pick the unnamed one).""" + entries = [ + _entry("a", "sk-or-a", priority=0), + _entry("b", "sk-or-b", priority=1, name="daily"), + ] + pool = _make_pool(entries, default_auth="daily", monkeypatch=monkeypatch) + assert pool.select().id == "b" + # Selection stays pinned across calls while the entry is healthy. + assert pool.select().id == "b" + + +def test_peek_prefers_named_credential(monkeypatch): + entries = [ + _entry("a", "sk-or-a", priority=0), + _entry("b", "sk-or-b", priority=1, name="daily"), + ] + pool = _make_pool(entries, default_auth="daily", monkeypatch=monkeypatch) + assert pool.peek().id == "b" + + +def test_select_falls_back_when_named_exhausted(monkeypatch): + """The named credential is exhausted → auto-rotate to the unnamed key.""" + entries = [ + _entry("a", "sk-or-a", priority=0), + _entry("b", "sk-or-b", priority=1, name="daily", exhausted=True), + ] + pool = _make_pool(entries, default_auth="daily", monkeypatch=monkeypatch) + assert pool.select().id == "a" + + +def test_select_falls_back_when_name_not_found(monkeypatch): + """default_auth names a credential that does not exist → legacy behavior.""" + entries = [ + _entry("a", "sk-or-a", priority=0), + _entry("b", "sk-or-b", priority=1), + ] + pool = _make_pool(entries, default_auth="ghost", monkeypatch=monkeypatch) + assert pool.select().id == "a" + + +def test_unnamed_pool_keeps_legacy_auto_rotate(monkeypatch): + """No default_auth configured → fill_first selection unchanged.""" + entries = [ + _entry("a", "sk-or-a", priority=0), + _entry("b", "sk-or-b", priority=1), + ] + pool = _make_pool(entries, default_auth=None, monkeypatch=monkeypatch) + assert pool.select().id == "a" + + +def test_mark_exhausted_rotates_off_named_onto_fallback(monkeypatch): + """Full loop: named key hits 429 → marked exhausted → next selection + falls back to the unnamed key instead of failing.""" + entries = [ + _entry("a", "sk-or-a", priority=0), + _entry("b", "sk-or-b", priority=1, name="daily"), + ] + pool = _make_pool(entries, default_auth="daily", monkeypatch=monkeypatch) + named = pool.select() + assert named.id == "b" + + rotated = pool.mark_exhausted_and_rotate(status_code=429, credential_id=named.id) + assert rotated.id == "a" + assert pool.select().id == "a" + + +def test_load_pool_honors_default_auth_end_to_end(tmp_path, monkeypatch): + """A pool loaded from disk with default_auth configured selects the named + entry, and re-selection after exhaustion lands on the unnamed fallback.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + monkeypatch.setattr( + "hermes_cli.auth._import_codex_cli_tokens", + lambda: None, + ) + _write_auth_store( + tmp_path, + { + "version": 1, + "credential_pool": { + "openrouter": [ + { + "id": "cred-a", + "label": "legacy", + "auth_type": "api_key", + "priority": 0, + "source": "manual", + "access_token": "sk-or-a", + }, + { + "id": "cred-b", + "label": "daily label", + "name": "daily", + "auth_type": "api_key", + "priority": 1, + "source": "manual", + "access_token": "sk-or-b", + }, + ] + }, + }, + ) + from agent import credential_pool as cp + + monkeypatch.setattr(cp, "_seed_from_singletons", lambda provider, entries: (False, set())) + monkeypatch.setattr(cp, "_seed_from_env", lambda provider, entries: (False, set())) + monkeypatch.setattr(cp, "get_default_auth_name", lambda _p: "daily") + + pool = cp.load_pool("openrouter") + assert pool.select().id == "cred-b" + + # Named entry payload survives a write cycle with its name intact. + persisted = json.loads((tmp_path / "hermes" / "auth.json").read_text()) + named_on_disk = next( + e for e in persisted["credential_pool"]["openrouter"] if e["id"] == "cred-b" + ) + assert named_on_disk["name"] == "daily" + + +def test_session_auth_env_var_overrides_default_auth(tmp_path, monkeypatch): + """HERMES_AUTH_NAME env var overrides config default_auth.""" + from agent import credential_pool as cp + + monkeypatch.setattr(cp, "_seed_from_env", lambda provider, entries: (False, set())) + monkeypatch.setattr(cp, "get_default_auth_name", lambda _p: "daily") + + monkeypatch.setenv("HERMES_AUTH_NAME", "monthly") + pool = cp.load_pool("openrouter") + assert pool._default_auth == "monthly" + + +def test_session_auth_env_var_empty_falls_back_to_config(tmp_path, monkeypatch): + """Empty HERMES_AUTH_NAME falls back to config default_auth.""" + from agent import credential_pool as cp + + monkeypatch.setattr(cp, "_seed_from_env", lambda provider, entries: (False, set())) + monkeypatch.setattr(cp, "get_default_auth_name", lambda _p: "daily") + + monkeypatch.setenv("HERMES_AUTH_NAME", "") + pool = cp.load_pool("openrouter") + assert pool._default_auth == "daily" + + +def test_acquire_lease_narrows_to_default_auth(monkeypatch): + """acquire_lease() respects default_auth name like select() does.""" + entries = [ + _entry("a", "sk-or-a", priority=0), + _entry("b", "sk-or-b", priority=1, name="daily"), + ] + pool = _make_pool(entries, default_auth="daily", monkeypatch=monkeypatch) + leased_id = pool.acquire_lease() + assert leased_id == "b" # entry b has name="daily" diff --git a/tests/hermes_cli/test_auth_commands.py b/tests/hermes_cli/test_auth_commands.py index 3da85849c0448..0b0e9576a8118 100644 --- a/tests/hermes_cli/test_auth_commands.py +++ b/tests/hermes_cli/test_auth_commands.py @@ -94,6 +94,57 @@ class _Args: assert entry["access_token"] == "sk-or-manual" +def test_auth_add_api_key_persists_name_for_manual_selection(tmp_path, monkeypatch): + """`hermes auth add --name ` stores the manual-selection name (#76937).""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + monkeypatch.delenv("OPENROUTER_API_KEY", raising=False) + monkeypatch.delenv("OPENAI_API_KEY", raising=False) + _write_auth_store(tmp_path, {"version": 1, "providers": {}}) + + from hermes_cli.auth_commands import auth_add_command + + class _Args: + provider = "openrouter" + auth_type = "api-key" + api_key = "sk-or-daily" + label = "daily key" + name = "daily" + + auth_add_command(_Args()) + + payload = json.loads((tmp_path / "hermes" / "auth.json").read_text()) + entries = payload["credential_pool"]["openrouter"] + entry = next(item for item in entries if item["source"] == "manual") + assert entry["name"] == "daily" + assert entry["access_token"] == "sk-or-daily" + + +def test_auth_add_without_name_leaves_no_name_key(tmp_path, monkeypatch): + """Unnamed keys keep the legacy payload shape (no ``name`` key).""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + monkeypatch.delenv("OPENROUTER_API_KEY", raising=False) + monkeypatch.delenv("OPENAI_API_KEY", raising=False) + _write_auth_store(tmp_path, {"version": 1, "providers": {}}) + + from hermes_cli.auth_commands import auth_add_command + + class _Args: + provider = "openrouter" + auth_type = "api-key" + api_key = "sk-or-legacy" + label = "legacy" + + auth_add_command(_Args()) + + payload = json.loads((tmp_path / "hermes" / "auth.json").read_text()) + entry = next( + item + for item in payload["credential_pool"]["openrouter"] + if item["source"] == "manual" + ) + assert "name" not in entry + + def test_auth_add_nous_oauth_persists_pool_entry(tmp_path, monkeypatch): monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) _write_auth_store(tmp_path, {"version": 1, "providers": {}}) diff --git a/tests/hermes_cli/test_skills_groups.py b/tests/hermes_cli/test_skills_groups.py new file mode 100644 index 0000000000000..30127899e9cd7 --- /dev/null +++ b/tests/hermes_cli/test_skills_groups.py @@ -0,0 +1,200 @@ +"""Tests for hermes_cli/skills_groups.py — skill group config, CLI helpers.""" + +from unittest.mock import patch + +from hermes_cli.skills_groups import ( + _validate_group_name, + add_skills_to_group, + get_skill_groups, + remove_skills_from_group, + save_skill_groups, +) + + +# --------------------------------------------------------------------------- +# get_skill_groups +# --------------------------------------------------------------------------- + +class TestGetSkillGroups: + def test_empty_config(self): + assert get_skill_groups({}) == {} + + def test_null_skills_section(self): + assert get_skill_groups({"skills": None}) == {} + + def test_null_groups_section(self): + assert get_skill_groups({"skills": {"groups": None}}) == {} + + def test_malformed_groups_section(self): + assert get_skill_groups({"skills": {"groups": "oops"}}) == {} + + def test_normalizes_scalars_lists_and_dedupes(self): + config = { + "skills": { + "groups": { + "security": ["web-pentest", "godmode", "web-pentest"], + "writing": "humanizer", + "empty": [], + " ": ["spaces"], + } + } + } + groups = get_skill_groups(config) + assert groups["security"] == ["godmode", "web-pentest"] + assert groups["writing"] == ["humanizer"] + assert "empty" not in groups + assert " " not in groups + + +# --------------------------------------------------------------------------- +# save_skill_groups +# --------------------------------------------------------------------------- + +class TestSaveSkillGroups: + @patch("hermes_cli.skills_groups.save_config") + def test_writes_sorted_unique_under_skills_groups(self, mock_save): + config = {} + save_skill_groups(config, {"security": ["godmode", "web-pentest", "godmode"]}) + assert config["skills"]["groups"] == { + "security": ["godmode", "web-pentest"] + } + mock_save.assert_called_once() + + @patch("hermes_cli.skills_groups.save_config") + def test_preserves_existing_skills_section(self, mock_save): + config = {"skills": {"disabled": ["old-skill"]}} + save_skill_groups(config, {"security": ["godmode"]}) + assert config["skills"]["disabled"] == ["old-skill"] + assert config["skills"]["groups"] == {"security": ["godmode"]} + + +# --------------------------------------------------------------------------- +# add_skills_to_group +# --------------------------------------------------------------------------- + +class TestAddSkillsToGroup: + @patch("hermes_cli.skills_groups.save_config") + def test_creates_group(self, mock_save): + config = {} + result = add_skills_to_group(config, "security", ["web-pentest", "godmode"]) + assert result["created"] is True + assert set(result["added"]) == {"web-pentest", "godmode"} + assert config["skills"]["groups"]["security"] == ["godmode", "web-pentest"] + + @patch("hermes_cli.skills_groups.save_config") + def test_appends_and_dedupes(self, mock_save): + config = {"skills": {"groups": {"security": ["godmode"]}}} + result = add_skills_to_group(config, "security", ["godmode", "humanizer"]) + assert result["duplicates"] == ["godmode"] + assert result["added"] == ["humanizer"] + assert config["skills"]["groups"]["security"] == ["godmode", "humanizer"] + + +# --------------------------------------------------------------------------- +# remove_skills_from_group +# --------------------------------------------------------------------------- + +class TestRemoveSkillsFromGroup: + @patch("hermes_cli.skills_groups.save_config") + def test_removes_skills(self, mock_save): + config = {"skills": {"groups": {"security": ["godmode", "web-pentest"]}}} + result = remove_skills_from_group(config, "security", ["godmode"]) + assert result["removed"] == ["godmode"] + assert result["group_deleted"] is False + assert config["skills"]["groups"]["security"] == ["web-pentest"] + + @patch("hermes_cli.skills_groups.save_config") + def test_deletes_group_when_empty(self, mock_save): + config = {"skills": {"groups": {"security": ["godmode"]}}} + result = remove_skills_from_group(config, "security", ["godmode"]) + assert result["group_deleted"] is True + assert "security" not in config["skills"]["groups"] + + @patch("hermes_cli.skills_groups.save_config") + def test_deletes_whole_group_without_skill_args(self, mock_save): + config = {"skills": {"groups": {"security": ["a", "b"]}}} + result = remove_skills_from_group(config, "security") + assert result["group_deleted"] is True + assert result["removed"] == ["a", "b"] + assert "security" not in config["skills"]["groups"] + + @patch("hermes_cli.skills_groups.save_config") + def test_unknown_group_is_noop(self, mock_save): + config = {} + result = remove_skills_from_group(config, "nope", ["x"]) + assert result["missing"] == ["x"] + assert result["group_deleted"] is False + mock_save.assert_not_called() + + +# --------------------------------------------------------------------------- +# _validate_group_name +# --------------------------------------------------------------------------- + +class TestValidateGroupName: + def test_rejects_empty(self): + assert _validate_group_name("") is not None + assert _validate_group_name(" ") is not None + + def test_rejects_whitespace(self): + assert _validate_group_name("my group") is not None + + def test_rejects_flag_like(self): + assert _validate_group_name("-security") is not None + + def test_accepts_valid_names(self): + assert _validate_group_name("security") is None + assert _validate_group_name("data-science") is None + assert _validate_group_name("writing2") is None + + +# --------------------------------------------------------------------------- +# CLI parser wiring +# --------------------------------------------------------------------------- + +def _build_parser(): + import argparse + + from hermes_cli.subcommands.skills import build_skills_parser + + parser = argparse.ArgumentParser(prog="hermes") + subparsers = parser.add_subparsers(dest="command") + build_skills_parser(subparsers, cmd_skills=lambda args: None) + return parser + + +class TestSkillsGroupParser: + def test_list_group_flag(self): + args = _build_parser().parse_args( + ["skills", "list", "--group", "security"] + ) + assert args.skills_action == "list" + assert args.group == "security" + + def test_group_add(self): + args = _build_parser().parse_args( + ["skills", "group", "add", "security", "web-pentest", "godmode"] + ) + assert args.skills_action == "group" + assert args.group_action == "add" + assert args.group == "security" + assert args.skills == ["web-pentest", "godmode"] + + def test_group_remove_whole_group(self): + args = _build_parser().parse_args( + ["skills", "group", "remove", "security"] + ) + assert args.group_action == "remove" + assert args.group == "security" + assert args.skills == [] + + def test_group_remove_skills(self): + args = _build_parser().parse_args( + ["skills", "group", "rm", "security", "godmode"] + ) + assert args.group_action == "rm" + assert args.skills == ["godmode"] + + def test_group_list_alias(self): + args = _build_parser().parse_args(["skills", "group", "ls"]) + assert args.group_action == "ls" diff --git a/tests/hermes_cli/test_skills_hub.py b/tests/hermes_cli/test_skills_hub.py index 8e087e2758fb4..23fe293bba098 100644 --- a/tests/hermes_cli/test_skills_hub.py +++ b/tests/hermes_cli/test_skills_hub.py @@ -70,6 +70,14 @@ def _capture(source_filter: str = "all") -> str: return sink.getvalue() +def _capture_group(group: str) -> str: + """Run do_list with a group filter into a string buffer.""" + sink = StringIO() + console = Console(file=sink, force_terminal=False, color_system=None) + do_list(group_filter=group, console=console) + return sink.getvalue() + + def _capture_check(monkeypatch, results, name=None) -> str: import tools.skills_hub as hub @@ -124,6 +132,37 @@ def _fake(platform=None): assert seen["platform"] is None +def test_do_list_group_filter(three_source_env, monkeypatch): + """`hermes skills list --group ` shows only skills in that group.""" + import hermes_cli.skills_groups as groups_mod + + monkeypatch.setattr( + groups_mod, + "get_skill_groups", + lambda config=None: {"security": ["hub-skill", "local-skill"]}, + ) + out = _capture_group("security") + assert "hub-skill" in out + assert "local-skill" in out + assert "builtin-skill" not in out + assert "group 'security'" in out + + +def test_do_list_unknown_group(three_source_env, monkeypatch): + """An unknown --group prints an error listing available groups.""" + import hermes_cli.skills_groups as groups_mod + + monkeypatch.setattr( + groups_mod, + "get_skill_groups", + lambda config=None: {"security": ["hub-skill"]}, + ) + out = _capture_group("nope") + assert "Unknown group 'nope'" in out + assert "Available groups: security" in out + assert "hub-skill" not in out + + # --------------------------------------------------------------------------- # Cross-registry hijack regression tests # @@ -313,3 +352,36 @@ def test_do_search_json_flag_emits_full_identifiers(capsys): # Table render must be suppressed — sink should be empty (no "Searching for:" header). assert "Searching for:" not in sink.getvalue() + +# --------------------------------------------------------------------------- +# Slash command --group parsing edge cases (#76985 salvage) +# --------------------------------------------------------------------------- + +def test_slash_list_group_missing_value(capsys, monkeypatch): + """`/skills list --group` (no value) prints an error, not unfiltered output.""" + import hermes_cli.skills_groups as groups_mod + + monkeypatch.setattr( + groups_mod, + "get_skill_groups", + lambda config=None: {"security": ["godmode"]}, + ) + handle_skills_slash("/skills list --group") + captured = capsys.readouterr().out + assert "requires a group name" in captured + + +def test_slash_list_group_flag_value(capsys, monkeypatch): + """`/skills list --group --source hub` rejects a flag-like value.""" + import hermes_cli.skills_groups as groups_mod + + monkeypatch.setattr( + groups_mod, + "get_skill_groups", + lambda config=None: {"security": ["godmode"]}, + ) + handle_skills_slash("/skills list --group --source hub") + captured = capsys.readouterr().out + assert "looks like a flag" in captured + assert "--source" in captured +