fix(skills): name existing categories when a category filter matches nothing - #90082
fix(skills): name existing categories when a category filter matches nothing#90082404Brothers wants to merge 1 commit into
Conversation
…nothing
skills_list() built its `categories` list from the already-filtered skill set,
so a filter that matched nothing returned {"skills": [], "categories": [],
"count": 0}. That reply cannot distinguish "the category you guessed is wrong"
from "no skills are installed", and it never names the category the skill is
actually installed under.
Measured 2026-08-07 on a live task: a model looking for a transcription skill
called skills_list(category="audio") while the skill was installed under
"media", read the empty reply as "the skill is missing", then wrote its own
script in /tmp and installed packages into an unrelated venv.
- Categories are now collected from the full set before filtering, and an empty
filtered result carries an explicit message telling the caller to check
`categories` and retry without the filter.
- `hermes skills inspect <name>` answered "No skill named X found in any
source" for a skill that is installed locally but offered by no hub source.
Formally true - hub commands search remote sources - but it reads as "this
skill does not exist". It now reports the local installation and its path,
while a genuinely unknown name still gets the not-found answer.
tests/tools/test_skills_tool.py locks both halves of the contract: a filter
miss must name every existing category and say so explicitly, and a filter hit
must stay clean so the warning keeps its signal.
alt-glitch
added
type/bug
Exactly the right fix for an agent-facing UX trap: computing No blocking issues found. Nit ( — reviewer-a · automated agent review (Hermes week-review) |
|
Thanks for the review! I’ll keep this PR focused on the current fix for now. I can address this edge case too if needed. |
4 participants
What does this PR do?
skills_list()built itscategorieslist from the already-filtered skill set, so acategory filter that matched nothing returned:
{"success": true, "skills": [], "categories": [], "count": 0}That reply cannot distinguish "the category you guessed is wrong" from "no skills are
installed", and it never names the category the skill is actually installed under. The
documented Level 0 contract is
skills_list() → [{name, description, category}, ...](Progressive Disclosure),
so
categoryis the only handle the agent has — and on a miss the reply took that handle away.Measured 2026-08-07 on a live task: a model looking for a transcription skill called
skills_list(category="audio")while the skill was installed undermedia, read the emptyreply as "the skill is missing", wrote its own script in
/tmpand installed packages intoan unrelated venv. The skill was installed and enabled the whole time.
The fix keeps the failure mode observable instead of silent: categories are computed before
filtering, and an empty filtered result says explicitly that this is not proof of absence.
Related Issue
No existing upstream issue or PR covers this. Searched before opening, as
CONTRIBUTING
asks:
Nearest hits are about index size and routing (#26786, #22620, #37227, #23663) and about empty
category directories on install (#34237) — different defect, different code path.
Type of Change
Changes Made
tools/skills_tool.py—categoriesis collected from the full skill set before thecategory filter is applied; an empty filtered result carries a
messagetelling the callerto check
categoriesand retry without the filter before concluding anything. A filter thatdid match stays untouched, so the message keeps its signal.
hermes_cli/skills_hub.py—hermes skills inspect <name>answeredNo skill named X found in any sourcefor a skill that is installed locally but offered byno hub source. Formally true (hub commands search remote sources) and misleading at once. It
now reports the local installation and its path; a genuinely unknown name still gets the
not-found answer. Same for short-name resolution (
_resolve_short_name). Shared helper_find_local_skill()accepts bothnameandcategory/name.tests/tools/test_skills_tool.py— two assertions lock the contract in both directions.How to Test
A filter miss must still name every existing category and say so explicitly; a filter hit
must stay clean. Against a real skills directory (125 skills, 20 categories):
Before this change the first line was
count=0 categories=0with no message — the exactinput the model in the 2026-08-07 incident acted on.
CLI half — installed skill vs invented name:
The invented name from the original incident is still honestly reported as absent.
Tests.
pytest tests/tools/test_skills_tool.py -q→ 43 passed (41 before, plus the twonew assertions). Widened to every skill-related test file in the tree, so both changed
modules are covered:
The two failures are pre-existing and unrelated —
test_skill_bundle_provenance.pypasses7 passedwhen run alone on both the branch and the base, so it is cross-test pollutioninside a shared pytest session, which is exactly what
scripts/run_tests.shavoids byisolating each file in its own subprocess. Net effect of this PR: +2 passing tests, nothing
else moved.
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass — ran the 88-file skill-related subsetinstead (see "How to Test" item 3); the full 3104-file suite is left to CI
Documentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/ANotes on the boxes above: docs are N/A because neither the
categoryparameter nor thecategoriesfield is described in theTools Reference
(
skills_listis documented as "List available skills (name + description)"), and thedocumented Level 0 shape is unchanged — only the miss path stops lying. No config keys, no
architecture change. The
skills_listtool schema is unchanged: same parameters, samefields;
messagewas already the field used for the "no skills at all" case, this reuses it.Cross-platform: both changes are pure string/dict work, and paths are built with
pathlib.