fix(skills): classify bundled skills by dir_name when frontmatter name differs#11
Draft
dsr-restyn wants to merge 4 commits into
Draft
fix(skills): classify bundled skills by dir_name when frontmatter name differs#11dsr-restyn wants to merge 4 commits into
dsr-restyn wants to merge 4 commits into
Conversation
/btw is designed to answer ephemeral side questions concurrently while an agent is working. When a running agent existed, the gateway was sending /btw as an interrupt to that agent (lines 1871-1876) and returning before ever reaching the command dispatch at line 1878. /approve and /deny already had this bypass pattern (they need it because the agent thread is blocked on a threading.Event). /btw needs the same treatment for the opposite reason: it should run concurrently, not interrupt. Fix: add /btw to the early-intercept check block, routing directly to _handle_btw_command regardless of running-agent state. Adds regression test: tests/gateway/test_btw_bypass.py
…e differs Fixes NousResearch#5433 in NousResearch/hermes-agent skills_sync stores bundled manifest entries by directory name (e.g. 'vllm'), but _find_all_skills() returns the SKILL.md frontmatter name (e.g. 'serving-llms-vllm'). When these differ, do_list() fails to match the skill against the manifest, misclassifying it as 'local' instead of 'builtin'. Fix: _find_all_skills() now includes 'dir_name' in each skill dict, and do_list() falls back to checking dir_name against the manifest when the frontmatter name is not found. No manifest migration required.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When a bundled skill's SKILL.md
namefrontmatter differs from its directory name,do_list()fails to recognise it as builtin and shows it as local instead.Root cause:
tools/skills_sync.pystores manifest entries keyed by directory name (e.g.vllm), whiletools/skills_tool._find_all_skills()returns the frontmatternamefield (e.g.serving-llms-vllm). The checkelif name in builtin_namesinhermes_cli/skills_hub.do_list()only compared frontmatter name, so mismatched entries always fell through to thelocalbranch.Fix:
_find_all_skills()now includesdir_name(the skill directory's basename) in each returned dict — a purely additive field.do_list()falls back toskill.get("dir_name") in builtin_nameswhen the frontmatter name doesn't match, covering all mismatch cases without requiring any manifest migration.Affected skills (per the issue report):
audiocraft-audio-generation,evaluating-llms-harness,fine-tuning-with-trl,gguf-quantization,modal-serverless-gpu,peft-fine-tuning,segment-anything-model,serving-llms-vllm,stable-diffusion-image-generation.Upstream Issue
Fixes NousResearch#5433
Testing
tests/hermes_cli/test_skills_hub.pytests pass.test_do_list_builtin_by_dir_name_when_frontmatter_differswhich directly reproduces the mismatch scenario (dir_name='vllm', frontmattername='serving-llms-vllm') and asserts the skill is classified as builtin, not local.