fix(skills): never call a rate-limited fetch a stale index entry; drop per-search caveat
A throttled GitHub fetch also yields index-metadata-without-bundle, so the new stale-entry verdict would tell users a skill "no longer exists upstream" when it does. Check the adapters' rate-limit flag first and keep the existing rate-limit hint for that case (the keep_open review concern on #3261). The per-search "results may be stale" note is dropped: it fires on every skills.sh search whether or not anything is stale, and the install-time error now names the condition precisely where it happens.
This commit is contained in:
@@ -341,13 +341,7 @@ def do_search(query: str, source: str = "all", limit: int = 10, console: Optiona
|
||||
c.print(table)
|
||||
c.print("[dim]Use: hermes skills inspect <identifier> to preview, "
|
||||
"hermes skills install <identifier> to install "
|
||||
"(--json for scripting)[/]")
|
||||
if any(r.source in ("skills.sh", "skills-sh") for r in results):
|
||||
c.print("[dim yellow]Note:[/] [dim]skills.sh results may include entries whose "
|
||||
"upstream files were removed or renamed; a 'stale index entry' error "
|
||||
"at install means the skill no longer exists there.[/]\n")
|
||||
else:
|
||||
c.print()
|
||||
"(--json for scripting)[/]\n")
|
||||
|
||||
|
||||
def _rank_and_page(all_results, page: int, page_size: int):
|
||||
@@ -592,18 +586,20 @@ def _pinned_sources(c: Console, sources, source_id: Optional[str], identifier: s
|
||||
|
||||
|
||||
def _print_fetch_failure(c: Console, sources, identifier: str, meta=None, source=None) -> None:
|
||||
# Index hit but files gone (GitHub 404): a stale index entry, not a user
|
||||
# typo — name it so users stop re-trying spellings (#3259).
|
||||
if meta is not None:
|
||||
rate_limited = any(getattr(src, "is_rate_limited", False)
|
||||
or getattr(getattr(src, "github", None), "is_rate_limited", False)
|
||||
for src in sources)
|
||||
# Index hit but files gone: a stale index entry, not a user typo — name it so users stop
|
||||
# re-trying spellings (#3259). Only when no adapter was rate limited: a throttled fetch
|
||||
# also yields meta-without-bundle, and calling that "stale" would send users away from a
|
||||
# skill that exists.
|
||||
if meta is not None and not rate_limited:
|
||||
src_id = getattr(source, "source_id", lambda: "the registry")()
|
||||
c.print(f"[bold red]Error:[/] '{identifier}' is listed in the {src_id} index, "
|
||||
f"but its files no longer exist upstream.")
|
||||
c.print("[dim]Stale index entry: the skill was likely renamed or removed by "
|
||||
"its author. Try `hermes skills search` for an alternative.[/]\n")
|
||||
return
|
||||
rate_limited = any(getattr(src, "is_rate_limited", False)
|
||||
or getattr(getattr(src, "github", None), "is_rate_limited", False)
|
||||
for src in sources)
|
||||
c.print(f"[bold red]Error:[/] Could not fetch '{identifier}' from any source.")
|
||||
if rate_limited:
|
||||
c.print("[yellow]Hint:[/] GitHub API rate limit exhausted "
|
||||
|
||||
@@ -536,41 +536,32 @@ def test_do_install_stale_index_names_the_problem(monkeypatch):
|
||||
assert "Could not fetch" not in out
|
||||
|
||||
|
||||
def test_do_install_unknown_identifier_stays_generic(monkeypatch):
|
||||
"""No index hit at all keeps the original generic message."""
|
||||
@pytest.mark.parametrize("meta_hit", [False, True])
|
||||
def test_do_install_generic_when_no_index_hit_or_rate_limited(monkeypatch, meta_hit):
|
||||
"""No index hit — or a throttled fetch that only *looks* like a stale entry — keeps the
|
||||
generic message (plus the rate-limit hint), never the stale-entry verdict."""
|
||||
import hermes_cli.skills_hub as cli_hub
|
||||
import tools.skills_hub as hub
|
||||
from hermes_cli.skills_hub import do_install
|
||||
|
||||
class ThrottledSource:
|
||||
is_rate_limited = meta_hit
|
||||
|
||||
def source_id(self):
|
||||
return "skills-sh"
|
||||
|
||||
meta = type("Meta", (), {"identifier": "skills-sh/org/gone-skill"})() if meta_hit else None
|
||||
src = ThrottledSource()
|
||||
monkeypatch.setattr(hub, "ensure_hub_dirs", lambda: None)
|
||||
monkeypatch.setattr(cli_hub, "_sources", lambda: [object()])
|
||||
monkeypatch.setattr(cli_hub, "_sources", lambda: [src])
|
||||
monkeypatch.setattr(
|
||||
cli_hub, "_resolve_source_meta_and_bundle",
|
||||
lambda identifier, sources: (None, None, None))
|
||||
lambda identifier, sources: (meta, None, src if meta_hit else None))
|
||||
sink = StringIO()
|
||||
console = Console(file=sink, force_terminal=False, color_system=None)
|
||||
do_install("nobody/nowhere/nothing", console=console, skip_confirm=True)
|
||||
do_install("skills-sh/org/gone-skill", console=console, skip_confirm=True)
|
||||
|
||||
out = sink.getvalue()
|
||||
assert "Could not fetch" in out
|
||||
assert "Stale index entry" not in out
|
||||
|
||||
|
||||
def test_do_search_warns_about_skills_sh_staleness(monkeypatch):
|
||||
"""Search results from skills.sh carry the stale-index caveat."""
|
||||
import tools.skills_hub_search as hub_search
|
||||
from hermes_cli.skills_hub import do_search
|
||||
import hermes_cli.skills_hub as cli_hub
|
||||
|
||||
row = type("Row", (), {
|
||||
"name": "gone-skill", "description": "d", "source": "skills-sh",
|
||||
"trust_level": "community",
|
||||
"identifier": "skills-sh/org/gone-skill"})()
|
||||
monkeypatch.setattr(cli_hub, "_sources", lambda: [])
|
||||
monkeypatch.setattr(hub_search, "unified_search",
|
||||
lambda query, sources, source_filter, limit: [row])
|
||||
sink = StringIO()
|
||||
console = Console(file=sink, force_terminal=False, color_system=None)
|
||||
do_search("gone", console=console)
|
||||
|
||||
assert "stale index entry" in sink.getvalue()
|
||||
assert ("rate limit" in out) is meta_hit
|
||||
|
||||
Reference in New Issue
Block a user