Conversation
Closes NousResearch#3259 When a skill appears in the skills.sh search index but its GitHub files no longer exist (renamed, deleted, or moved by the author), the user previously received only a generic 'Could not fetch' error with no explanation. Changes: - do_install: when _resolve_source_meta_and_bundle returns metadata (index hit) but no bundle (GitHub 404), show a 'stale index entry' message explaining the file no longer exists at its listed path and may have been renamed or removed. The generic 'Could not fetch' message is preserved for unknown identifiers where even the index has no record. - do_search: when results include entries from the skills.sh source, append a dim Note explaining that the skills.sh index may contain entries whose GitHub files have since been removed, so a 'stale index entry' error during install means the skill is gone -- not a user error. Official-only results are not annotated. 4 new tests covering both error paths and both search annotation cases.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for improving the skills-hub failure message. The generic failure remains on current main at hermes_cli/skills_hub.py:545, so the underlying UX problem is still valid.
Problems
- The new
meta is not Nonebranch athermes_cli/skills_hub.py:343is not sufficient proof of a GitHub 404. On current main,HermesIndexSource.inspect()returns index metadata without fetching (tools/skills_hub.py:3886-3892), whileGitHubSource.fetch()maps all download failures toNone(tools/skills_hub.py:592-606). This can call a rate-limit or transport failure a stale entry. - Current
do_install()retains a rate-limit hint athermes_cli/skills_hub.py:539-553(added by7e0e5ea03). The PR branch predates that path, so the stale branch needs to preserve it.
Suggested changes
- Carry a confirmed fetch status/reason through resolution, and reserve the stale-index message for a matching confirmed 404.
- Add 404, rate-limit, and generic-fetch-failure cases; the current fake source at
tests/hermes_cli/test_skills_hub.py:258returns bareNone, so it cannot validate the proposed distinction.
Automated hermes-sweeper review.
|
|
||
| if not bundle: | ||
| c.print(f"[bold red]Error:[/] Could not fetch '{identifier}' from any source.\n") | ||
| if meta is not None: |
There was a problem hiding this comment.
meta only proves that some source returned metadata. On current main the centralized index returns metadata without fetching, while GitHub fetch reduces 404s, rate limits, and other failures to None; please gate this message on a confirmed matching 404 and retain the existing rate-limit hint.
_resolve_source_meta_and_bundle already distinguishes index-hit-without- files from unknown identifiers, but do_install printed the same generic 'Could not fetch' for both, sending users off to re-check spellings for what is actually a stale skills.sh entry. Split the message, and add a staleness caveat to do_search results from skills.sh. Fixes #3259. Supersedes #3261 (stale since July — re-applied onto the current _print_fetch_failure helper).
…p 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.
|
Closing — superseded by #108906 ( |
Closes #3259
Problem
The skills.sh index contains entries (
vercel-react-best-practices,react-email,react-pdf, etc.) that appear in search results with high install counts but whose GitHub files no longer exist. When a user tries to install one, they get:This looks like a user error (typo, wrong identifier) but is actually an index synchronisation issue on skills.sh's side — the skill was removed or renamed by its author.
Root cause
_resolve_source_meta_and_bundle()already distinguishes the two cases:meta != None, bundle == None→ index hit, GitHub 404 (stale entry)meta == None, bundle == None→ unknown identifierBut
do_installused the same generic error message for both.Changes
hermes_cli/skills_hub.pydo_install— split the error branch:metafound, nobundle): named error explaining the index entry is stale, that the skill may have been renamed or removed, and that this is not a user error.meta, nobundle): original generic "Could not fetch" message preserved.do_search— add a caveat footnote when results include skills.sh entries, explaining that the index may contain entries whose GitHub files no longer exist, and what to expect if install fails.Before / After
Before:
After:
And in search results containing skills.sh entries:
Tests
4 new unit tests:
test_do_install_stale_index_shows_helpful_message— meta found, bundle None → stale messagetest_do_install_unknown_identifier_shows_generic_message— both None → generic message, no staletest_do_search_shows_skills_sh_caveat— results with skills-sh source → caveat showntest_do_search_no_caveat_for_official_only— official-only results → no caveat