From e0572a6def17fa359e00691be76421eccb82b5ee Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 28 May 2026 04:53:13 -0700 Subject: [PATCH] fix(skills-hub): stop ellipsis-truncating the Identifier column (#33810) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `hermes skills search` rendered the Identifier column with the default overflow behaviour, so long slugs (notably browse-sh — every browse-sh skill ends in a `-XXXXXX` hash that's part of the identifier) were cut to `browse-sh/weathe…`. Users copied the visible string into `hermes skills install` and got a not-found error because the hash was gone. Set overflow="fold" on the Identifier column in both search tables (`do_search` and the `_resolve_short_name` multi-match table) so long slugs wrap onto a second line instead of getting eaten. Also add a `--json` flag to `hermes skills search` (and the `/skills search` slash variant) for scripting — emits a list of {name, identifier, source, trust_level, description} objects with the full identifier, which is the right shape for copy-paste pipelines too. Closes #33674. --- hermes_cli/main.py | 5 ++ hermes_cli/skills_hub.py | 55 ++++++++++++++--- tests/hermes_cli/test_skills_hub.py | 92 +++++++++++++++++++++++++++++ 3 files changed, 143 insertions(+), 9 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 4490d59cbd6..0de49eaeaef 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -12656,6 +12656,11 @@ Examples: ], ) skills_search.add_argument("--limit", type=int, default=10, help="Max results") + skills_search.add_argument( + "--json", + action="store_true", + help="Output JSON instead of a table (full identifiers, scripting-friendly)", + ) skills_install = skills_subparsers.add_parser("install", help="Install a skill") skills_install.add_argument( diff --git a/hermes_cli/skills_hub.py b/hermes_cli/skills_hub.py index b617b69f384..4fe2a4dc7d8 100644 --- a/hermes_cli/skills_hub.py +++ b/hermes_cli/skills_hub.py @@ -58,7 +58,9 @@ def _resolve_short_name(name: str, sources, console: Console) -> str: table = Table() table.add_column("Source", style="dim") table.add_column("Trust", style="dim") - table.add_column("Identifier", style="bold cyan") + # overflow="fold" keeps the full slug visible (wraps instead of ellipsis-truncating) + # so users can copy it for `hermes skills install`. + table.add_column("Identifier", style="bold cyan", overflow="fold", no_wrap=False) for r in exact: trust_style = {"builtin": "bright_cyan", "trusted": "green", "community": "yellow"}.get(r.trust_level, "dim") trust_label = "official" if r.source == "official" else r.trust_level @@ -244,15 +246,39 @@ def _prompt_for_category(c: Console, existing: List[str]) -> str: def do_search(query: str, source: str = "all", limit: int = 10, - console: Optional[Console] = None) -> None: - """Search registries and display results as a Rich table.""" + console: Optional[Console] = None, as_json: bool = False) -> None: + """Search registries and display results as a Rich table. + + When ``as_json=True`` writes a JSON array of result records to stdout + (one object per skill: ``name``, ``identifier``, ``source``, + ``trust_level``, ``description``) and skips the table render. This is + the scripting / copy-paste handle: the full identifier is always + intact, even for browse-sh slugs that the table would otherwise wrap. + """ from tools.skills_hub import GitHubAuth, create_source_router, unified_search c = console or _console - c.print(f"\n[bold]Searching for:[/] {query}") auth = GitHubAuth() sources = create_source_router(auth) + if as_json: + # Avoid Rich status spinner contaminating stdout — JSON consumers + # expect a clean parseable stream. + results = unified_search(query, sources, source_filter=source, limit=limit) + payload = [ + { + "name": r.name, + "identifier": r.identifier, + "source": r.source, + "trust_level": r.trust_level, + "description": r.description, + } + for r in results + ] + print(json.dumps(payload, indent=2)) + return + + c.print(f"\n[bold]Searching for:[/] {query}") with c.status("[bold]Searching registries..."): results = unified_search(query, sources, source_filter=source, limit=limit) @@ -265,7 +291,11 @@ def do_search(query: str, source: str = "all", limit: int = 10, table.add_column("Description", max_width=60) table.add_column("Source", style="dim") table.add_column("Trust", style="dim") - table.add_column("Identifier", style="dim") + # overflow="fold" keeps the full slug visible (wraps instead of + # ellipsis-truncating). Browse.sh slugs end in a `-XXXXXX` hash that + # is part of the actual identifier — truncating it makes copy-paste + # into `hermes skills install` fail. + table.add_column("Identifier", style="dim", overflow="fold", no_wrap=False) for r in results: trust_style = {"builtin": "bright_cyan", "trusted": "green", "community": "yellow"}.get(r.trust_level, "dim") @@ -280,7 +310,8 @@ def do_search(query: str, source: str = "all", limit: int = 10, c.print(table) c.print("[dim]Use: hermes skills inspect to preview, " - "hermes skills install to install[/]\n") + "hermes skills install to install " + "(--json for scripting)[/]\n") def do_browse(page: int = 1, page_size: int = 20, source: str = "all", @@ -1390,7 +1421,8 @@ def skills_command(args) -> None: if action == "browse": do_browse(page=args.page, page_size=args.size, source=args.source) elif action == "search": - do_search(args.query, source=args.source, limit=args.limit) + do_search(args.query, source=args.source, limit=args.limit, + as_json=getattr(args, "json", False)) elif action == "install": do_install(args.identifier, category=args.category, force=args.force, skip_confirm=getattr(args, "yes", False), @@ -1511,10 +1543,11 @@ def handle_skills_slash(cmd: str, console: Optional[Console] = None) -> None: elif action == "search": if not args: - c.print("[bold red]Usage:[/] /skills search [--source skills-sh|well-known|github|official] [--limit N]\n") + c.print("[bold red]Usage:[/] /skills search [--source skills-sh|well-known|github|official] [--limit N] [--json]\n") return source = "all" limit = 10 + as_json = False query_parts = [] i = 0 while i < len(args): @@ -1527,10 +1560,14 @@ def handle_skills_slash(cmd: str, console: Optional[Console] = None) -> None: except ValueError: pass i += 2 + elif args[i] == "--json": + as_json = True + i += 1 else: query_parts.append(args[i]) i += 1 - do_search(" ".join(query_parts), source=source, limit=limit, console=c) + do_search(" ".join(query_parts), source=source, limit=limit, + console=c, as_json=as_json) elif action == "install": if not args: diff --git a/tests/hermes_cli/test_skills_hub.py b/tests/hermes_cli/test_skills_hub.py index 25798dcd3eb..1e505cd758c 100644 --- a/tests/hermes_cli/test_skills_hub.py +++ b/tests/hermes_cli/test_skills_hub.py @@ -651,3 +651,95 @@ def test_browse_skills_dedup_uses_identifier_not_name(monkeypatch): "browse_skills() must not deduplicate browse-sh skills with the same name " "but different identifiers" ) + + +# --------------------------------------------------------------------------- +# Regression: full identifier must be recoverable from `hermes skills search` +# even when the slug is too long to fit the terminal width (issue #33674). +# --------------------------------------------------------------------------- + +# A real browse-sh-style slug whose trailing -XXXXXX hash matters for install +_LONG_SLUG = "browse-sh/weather.gov/get-forecast-1uezib" + +_LONG_RESULT = type("R", (), { + "name": "get-forecast", + "description": "Fetch the forecast", + "source": "browse-sh", + "trust_level": "community", + "identifier": _LONG_SLUG, +})() + + +def test_do_search_identifier_column_does_not_truncate_long_slug(): + """The Identifier column must use overflow='fold', not the default ellipsis. + + Renders into a deliberately narrow Console; the full slug (including the + trailing -1uezib hash) must still appear in the output. Before the fix, + Rich would render `browse-sh/weather…` and lose the hash. + """ + from hermes_cli.skills_hub import do_search + + sink = StringIO() + # Narrow width forces Rich to apply overflow rules — exactly the scenario + # the issue reports. width=40 is too small for the slug; we want the slug + # wrapped (not ellipsis-truncated). + console = Console(file=sink, force_terminal=False, color_system=None, width=40) + + with patch("tools.skills_hub.unified_search", return_value=[_LONG_RESULT]), \ + patch("tools.skills_hub.create_source_router", return_value={}), \ + patch("tools.skills_hub.GitHubAuth"): + do_search("weather", console=console) + + output = sink.getvalue() + + # The fix is working when the Identifier column wraps the slug across + # multiple lines (folded chunks) rather than emitting ONE line with an + # ellipsis. Extract every chunk that appears in the rightmost cell of + # the table by walking lines that look like table rows ("│ ... │") and + # taking the last `│...│` cell. Concatenating those chunks must yield + # the full slug. + chunks = [] + for line in output.splitlines(): + # Table data rows start and end with the box-drawing vertical bar. + if not line.startswith("│") or not line.rstrip().endswith("│"): + continue + # Last `│ ... │` cell on the row is the Identifier column. + last_cell = line.rstrip().rsplit("│", 2)[-2].strip() + if last_cell: + chunks.append(last_cell) + reconstructed = "".join(chunks) + assert _LONG_SLUG in reconstructed, ( + f"Expected full slug {_LONG_SLUG!r} to be recoverable from the " + f"folded Identifier column; got chunks {chunks!r}\n" + f"Full output:\n{output}" + ) + # And the truncating ellipsis must NOT appear in the Identifier column. + # Rich uses U+2026 HORIZONTAL ELLIPSIS for the default overflow="ellipsis". + assert "\u2026" not in reconstructed, ( + f"Identifier column still ellipsis-truncated: {reconstructed!r}" + ) + + +def test_do_search_json_flag_emits_full_identifiers(capsys): + """`--json` must print a parseable array with full identifiers and skip the table.""" + from hermes_cli.skills_hub import do_search + + sink = StringIO() + console = Console(file=sink, force_terminal=False, color_system=None, width=40) + + with patch("tools.skills_hub.unified_search", return_value=[_LONG_RESULT]), \ + patch("tools.skills_hub.create_source_router", return_value={}), \ + patch("tools.skills_hub.GitHubAuth"): + do_search("weather", console=console, as_json=True) + + # JSON goes to stdout via print(), not the Rich console sink. + captured = capsys.readouterr().out + import json as _json + payload = _json.loads(captured) + assert isinstance(payload, list) and len(payload) == 1 + assert payload[0]["identifier"] == _LONG_SLUG + assert payload[0]["name"] == "get-forecast" + assert payload[0]["source"] == "browse-sh" + # Table render must be suppressed — sink should be empty (no "Searching for:" header). + assert "Searching for:" not in sink.getvalue() +