mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-29 06:31:32 +00:00
fix(skills-hub): stop ellipsis-truncating the Identifier column (#33810)
`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.
This commit is contained in:
parent
5e1f793430
commit
e0572a6def
3 changed files with 143 additions and 9 deletions
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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 <identifier> to preview, "
|
||||
"hermes skills install <identifier> to install[/]\n")
|
||||
"hermes skills install <identifier> 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 <query> [--source skills-sh|well-known|github|official] [--limit N]\n")
|
||||
c.print("[bold red]Usage:[/] /skills search <query> [--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:
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue