feat: expose skills RPCs - #19168
Conversation
alt-glitch
added
comp/tui
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for exposing a useful native-client surface. The exact five RPCs are not present on current main, so the capability remains relevant, but this implementation needs to align with current skill infrastructure.
Problems
tui_gateway/server.py:3973captures module-levelSKILLS_DIR; current main uses call-time resolution for multi-profile TUI/Desktop backends (tools/skill_manager_tool.py:156-168,c6a3d412d).tui_gateway/server.py:3975makes all external roots read-only, contrary to the documented foreground-edit behavior for writableskills.external_dirs(website/docs/user-guide/features/skills.md:325-328).tui_gateway/server.py:4201-4202bypasses the manager write path and does not clear the skills prompt snapshot; the dashboard delegates to_edit_skillthen invalidates that cache (hermes_cli/web_server.py:13267-13279).- The recursive scan RPCs are not in
_LONG_HANDLERS, so they run inline viadispatch()(tui_gateway/server.py:1254-1290).
Suggested changes
- Reuse the canonical skill manager and call-time profile-scoped root resolution.
- Preserve writable external directories for foreground edits.
- Pool scan RPCs and add protocol tests for profile, external-root, and update/cache behavior.
Automated hermes-sweeper review.
| from agent.skill_utils import get_external_skills_dirs | ||
| from tools.skills_tool import SKILLS_DIR | ||
|
|
||
| roots: list[tuple[Path, str, bool]] = [(Path(SKILLS_DIR), "local", True)] |
There was a problem hiding this comment.
Do not capture SKILLS_DIR here. Current main resolved this bug class by using the active HERMES_HOME at call time for long-lived TUI/Desktop backends (tools/skill_manager_tool.py:156-168, c6a3d412d); this local root can otherwise target the launch profile rather than the active profile.
|
|
||
| roots: list[tuple[Path, str, bool]] = [(Path(SKILLS_DIR), "local", True)] | ||
| for idx, root in enumerate(get_external_skills_dirs()): | ||
| roots.append((Path(root), f"external:{idx}", False)) |
There was a problem hiding this comment.
Marking every external root read-only changes the current foreground-edit contract. website/docs/user-guide/features/skills.md:325-328 documents in-place updates for writable skills.external_dirs; only autonomous curation treats those roots as read-only.
| _validate_skill_content(content) | ||
| target = skill_dir / "SKILL.md" | ||
| tmp = target.with_suffix(".md.tmp") | ||
| tmp.write_text(content, encoding="utf-8") |
There was a problem hiding this comment.
Route this through the canonical skill-manager rewrite path instead of writing directly. Current dashboard updates use _edit_skill and clear the skills prompt cache afterward (hermes_cli/web_server.py:13267-13279); this direct write omits those established behaviors.
| raise ValueError("skill description must be <= 1024 characters") | ||
|
|
||
|
|
||
| @method("skills.list") |
There was a problem hiding this comment.
These root-scanning RPCs need registration in _LONG_HANDLERS; otherwise dispatch() runs them on the reader thread. Current main specifically pools skills.manage to prevent skill scans and network-backed actions from stalling inbound TUI RPCs.
teknium1
added
sweeper:risk-compatibility
3 participants
Summary
Exposes Hermes Agent skills over the TUI/API-server JSON-RPC transport for native clients.
skills.list,skills.tree,skills.get,skills.graph, andskills.update$HERMES_HOME/skillsplus configured external skill dirsSKILL.mdfrontmatter before writesskills.changedafter updateValidation
python3 -m py_compile tui_gateway/server.pyskills.listskills.treeskills.graphskills.getskills.updateNotes
Companion HermesNative PR adds the UI that consumes these RPCs.