From a09343cc964ba462dce871d7ebd3f225314d3ca7 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 11 Jun 2026 06:10:27 -0700 Subject: [PATCH] feat(dashboard): SKILL.md editor on Skills page + attach-skill selector in cron modals (#44231) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Headless/VPS users (dashboard-over-Tailscale, no comfortable SSH) could list/toggle/install skills and create/edit cron jobs, but not author a custom skill or link one to a cron job — the UI set WHEN a job runs, but not WHICH skill it uses. - Skills page: 'New skill' button + per-row edit pencil open a SKILL.md editor dialog (frontmatter + body, server-side validation via the same _create_skill/_edit_skill path as the agent's skill_manage tool). - New endpoints: GET /api/skills/content, POST /api/skills, PUT /api/skills/content — all profile-scoped via _profile_scope(), which now also retargets tools.skill_manager_tool's import-time SKILLS_DIR binding. - Cron page: skills multi-select in both create and edit modals (parity with hermes cron --skill / edit --add-skill); CronJobCreate gains a skills field; job cards show an attached-skills badge. update_job already accepted skills in updates. - Tests: 17 new endpoint tests (content read, create/edit validation + profile scoping + auth gate, cron skills round-trip). --- hermes_cli/web_server.py | 127 ++++++++- .../test_web_server_skill_editor.py | 259 ++++++++++++++++++ web/src/components/SkillEditorDialog.tsx | 215 +++++++++++++++ web/src/lib/api.ts | 34 ++- web/src/pages/CronPage.tsx | 123 ++++++++- web/src/pages/SkillsPage.tsx | 70 ++++- 6 files changed, 806 insertions(+), 22 deletions(-) create mode 100644 tests/hermes_cli/test_web_server_skill_editor.py create mode 100644 web/src/components/SkillEditorDialog.tsx diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 0a35d93e607..2d00ca28128 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -6248,6 +6248,7 @@ class CronJobCreate(BaseModel): schedule: str name: str = "" deliver: str = "local" + skills: Optional[List[str]] = None class CronJobUpdate(BaseModel): @@ -6419,6 +6420,7 @@ async def create_cron_job(body: CronJobCreate, profile: str = "default"): schedule=body.schedule, name=body.name, deliver=body.deliver, + skills=body.skills, ) except Exception as e: _log.exception("POST /api/cron/jobs failed") @@ -8649,36 +8651,54 @@ def _profile_scope(profile: Optional[str]): 1. ``load_config``/``save_config`` resolve ``get_hermes_home()`` at call time — the context-local override from ``set_hermes_home_override`` reaches them (same pattern as ``_write_profile_model``). - 2. ``tools.skills_tool`` binds ``SKILLS_DIR`` at import time, so the - override CANNOT reach it. Like ``_call_cron_for_profile`` does for - cron's module globals, temporarily retarget it under a lock and - restore it immediately after. + 2. ``tools.skills_tool`` and ``tools.skill_manager_tool`` bind + ``SKILLS_DIR`` at import time, so the override CANNOT reach them. + Like ``_call_cron_for_profile`` does for cron's module globals, + temporarily retarget both under a lock and restore them + immediately after. ``profile`` of None/""/"current" means "the dashboard's own profile" — - a no-op scope, preserving existing behavior for old clients. + config resolution is untouched, but the skill-module globals are still + retargeted to the *current* ``get_hermes_home()`` so writes land in the + live home even when the import-time binding is stale (e.g. the process + imported the modules before a HERMES_HOME override, or under test + isolation). """ requested = (profile or "").strip() - if not requested or requested.lower() == "current": - yield None - return - profile_dir = _resolve_profile_dir(requested) - - from hermes_constants import set_hermes_home_override, reset_hermes_home_override + from hermes_constants import ( + get_hermes_home, + set_hermes_home_override, + reset_hermes_home_override, + ) from tools import skills_tool as _skills_tool + from tools import skill_manager_tool as _skill_mgr + + token = None + if not requested or requested.lower() == "current": + profile_dir = get_hermes_home() + else: + profile_dir = _resolve_profile_dir(requested) + token = set_hermes_home_override(str(profile_dir)) - token = set_hermes_home_override(str(profile_dir)) with _SKILLS_PROFILE_LOCK: old_home = _skills_tool.HERMES_HOME old_skills_dir = _skills_tool.SKILLS_DIR + old_mgr_home = _skill_mgr.HERMES_HOME + old_mgr_skills_dir = _skill_mgr.SKILLS_DIR _skills_tool.HERMES_HOME = profile_dir _skills_tool.SKILLS_DIR = profile_dir / "skills" + _skill_mgr.HERMES_HOME = profile_dir + _skill_mgr.SKILLS_DIR = profile_dir / "skills" try: - yield profile_dir + yield profile_dir if token is not None else None finally: _skills_tool.HERMES_HOME = old_home _skills_tool.SKILLS_DIR = old_skills_dir - reset_hermes_home_override(token) + _skill_mgr.HERMES_HOME = old_mgr_home + _skill_mgr.SKILLS_DIR = old_mgr_skills_dir + if token is not None: + reset_hermes_home_override(token) class SkillToggle(BaseModel): @@ -8714,6 +8734,85 @@ async def toggle_skill(body: SkillToggle, profile: Optional[str] = None): return {"ok": True, "name": body.name, "enabled": body.enabled} +class SkillCreate(BaseModel): + name: str + content: str + category: Optional[str] = None + profile: Optional[str] = None + + +class SkillContentUpdate(BaseModel): + name: str + content: str + profile: Optional[str] = None + + +def _clear_skills_prompt_cache() -> None: + """Best-effort: invalidate the skills system-prompt snapshot after a write. + + Mirrors what ``skill_manage`` does so a dashboard-authored skill is picked + up by the next session without a manual cache reset. + """ + try: + from agent.prompt_builder import clear_skills_system_prompt_cache + clear_skills_system_prompt_cache(clear_snapshot=True) + except Exception: + pass + + +@app.get("/api/skills/content") +async def get_skill_content(name: str, profile: Optional[str] = None): + """Return the raw SKILL.md text for a skill, for the dashboard editor.""" + from tools.skill_manager_tool import _find_skill + + with _profile_scope(profile): + found = _find_skill(name) + if not found: + raise HTTPException(status_code=404, detail=f"Skill '{name}' not found.") + skill_md = found["path"] / "SKILL.md" + if not skill_md.exists(): + raise HTTPException(status_code=404, detail=f"Skill '{name}' has no SKILL.md.") + try: + content = skill_md.read_text(encoding="utf-8") + except OSError as exc: + raise HTTPException(status_code=500, detail=str(exc)) from exc + return {"name": name, "content": content, "path": str(skill_md)} + + +@app.post("/api/skills") +async def create_skill(body: SkillCreate): + """Create a new custom skill (SKILL.md) from the dashboard editor. + + Calls the same validated write path as the agent's ``skill_manage`` + tool (frontmatter validation, name/category validation, size limit, + optional security scan) — but bypasses the agent write-approval gate: + a write from the authenticated dashboard IS the user acting directly. + """ + from tools.skill_manager_tool import _create_skill + + with _profile_scope(body.profile): + result = _create_skill(body.name, body.content, body.category or None) + if not result.get("success"): + raise HTTPException(status_code=400, detail=result.get("error", "Failed to create skill.")) + _clear_skills_prompt_cache() + return result + + +@app.put("/api/skills/content") +async def update_skill_content(body: SkillContentUpdate): + """Replace the SKILL.md of an existing skill (full rewrite) from the editor.""" + from tools.skill_manager_tool import _edit_skill + + with _profile_scope(body.profile): + result = _edit_skill(body.name, body.content) + if not result.get("success"): + err = result.get("error", "Failed to update skill.") + status = 404 if "not found" in str(err).lower() else 400 + raise HTTPException(status_code=status, detail=err) + _clear_skills_prompt_cache() + return result + + @app.get("/api/tools/toolsets") async def get_toolsets(profile: Optional[str] = None): from hermes_cli.tools_config import ( diff --git a/tests/hermes_cli/test_web_server_skill_editor.py b/tests/hermes_cli/test_web_server_skill_editor.py new file mode 100644 index 00000000000..c89142ae0df --- /dev/null +++ b/tests/hermes_cli/test_web_server_skill_editor.py @@ -0,0 +1,259 @@ +"""Tests for the dashboard skill editor endpoints and cron skill attachment. + +The Skills page can now create/edit custom skills (SKILL.md) and the Cron +page can attach skills to jobs — closing the "SSH + nano is the only way" +gap for headless/VPS users. These tests pin: + +- GET /api/skills/content returns raw SKILL.md (and profile-scopes). +- POST /api/skills creates a skill through the same validated write path + as the agent's ``skill_manage`` tool (frontmatter validation enforced). +- PUT /api/skills/content rewrites an existing SKILL.md (404 on unknown). +- POST /api/cron/jobs accepts ``skills`` and persists it on the job; + PUT /api/cron/jobs/{id} can update the list. +""" +import pytest + + +SKILL_MD = """--- +name: {name} +description: a test skill +--- + +# {name} + +Do the thing. +""" + + +def _write_skill(skills_dir, name): + d = skills_dir / name + d.mkdir(parents=True, exist_ok=True) + (d / "SKILL.md").write_text(SKILL_MD.format(name=name), encoding="utf-8") + + +@pytest.fixture +def isolated_profiles(tmp_path, monkeypatch, _isolate_hermes_home): + """Isolated default home + one named profile, each with its own skills.""" + from hermes_constants import get_hermes_home + from hermes_cli import profiles + + default_home = get_hermes_home() + profiles_root = default_home / "profiles" + worker_home = profiles_root / "worker_alpha" + for home in (default_home, worker_home): + (home / "skills").mkdir(parents=True, exist_ok=True) + (home / "config.yaml").write_text("{}\n", encoding="utf-8") + + _write_skill(default_home / "skills", "dashboard-skill") + _write_skill(worker_home / "skills", "worker-skill") + + monkeypatch.setattr(profiles, "_get_default_hermes_home", lambda: default_home) + monkeypatch.setattr(profiles, "_get_profiles_root", lambda: profiles_root) + return {"default": default_home, "worker_alpha": worker_home} + + +@pytest.fixture +def client(monkeypatch, isolated_profiles): + try: + from starlette.testclient import TestClient + except ImportError: + pytest.skip("fastapi/starlette not installed") + + import hermes_state + from hermes_constants import get_hermes_home + from hermes_cli.web_server import app, _SESSION_HEADER_NAME, _SESSION_TOKEN + + monkeypatch.setattr(hermes_state, "DEFAULT_DB_PATH", get_hermes_home() / "state.db") + c = TestClient(app) + c.headers[_SESSION_HEADER_NAME] = _SESSION_TOKEN + return c + + +class TestSkillContent: + def test_get_content_returns_raw_skill_md(self, client, isolated_profiles): + resp = client.get("/api/skills/content", params={"name": "dashboard-skill"}) + assert resp.status_code == 200 + data = resp.json() + assert data["name"] == "dashboard-skill" + assert data["content"].startswith("---") + assert "Do the thing." in data["content"] + + def test_get_content_scopes_to_profile(self, client, isolated_profiles): + resp = client.get( + "/api/skills/content", + params={"name": "worker-skill", "profile": "worker_alpha"}, + ) + assert resp.status_code == 200 + # ...and the worker skill is invisible without the profile param. + resp = client.get("/api/skills/content", params={"name": "worker-skill"}) + assert resp.status_code == 404 + + def test_get_content_unknown_skill_404(self, client, isolated_profiles): + resp = client.get("/api/skills/content", params={"name": "nope"}) + assert resp.status_code == 404 + + +class TestSkillCreate: + def test_create_writes_skill_md(self, client, isolated_profiles): + resp = client.post( + "/api/skills", + json={"name": "my-new-skill", "content": SKILL_MD.format(name="my-new-skill")}, + ) + assert resp.status_code == 200 + assert resp.json()["success"] is True + skill_md = isolated_profiles["default"] / "skills" / "my-new-skill" / "SKILL.md" + assert skill_md.exists() + assert "Do the thing." in skill_md.read_text(encoding="utf-8") + + def test_create_with_category(self, client, isolated_profiles): + resp = client.post( + "/api/skills", + json={ + "name": "cat-skill", + "category": "devops", + "content": SKILL_MD.format(name="cat-skill"), + }, + ) + assert resp.status_code == 200 + assert ( + isolated_profiles["default"] / "skills" / "devops" / "cat-skill" / "SKILL.md" + ).exists() + + def test_create_scopes_to_profile(self, client, isolated_profiles): + resp = client.post( + "/api/skills", + json={ + "name": "worker-new", + "content": SKILL_MD.format(name="worker-new"), + "profile": "worker_alpha", + }, + ) + assert resp.status_code == 200 + assert ( + isolated_profiles["worker_alpha"] / "skills" / "worker-new" / "SKILL.md" + ).exists() + # Dashboard's own skills dir stays clean. + assert not ( + isolated_profiles["default"] / "skills" / "worker-new" + ).exists() + + def test_create_rejects_missing_frontmatter(self, client, isolated_profiles): + resp = client.post( + "/api/skills", + json={"name": "bad-skill", "content": "no frontmatter here"}, + ) + assert resp.status_code == 400 + assert "frontmatter" in resp.json()["detail"].lower() + assert not (isolated_profiles["default"] / "skills" / "bad-skill").exists() + + def test_create_rejects_duplicate_name(self, client, isolated_profiles): + resp = client.post( + "/api/skills", + json={ + "name": "dashboard-skill", + "content": SKILL_MD.format(name="dashboard-skill"), + }, + ) + assert resp.status_code == 400 + assert "already exists" in resp.json()["detail"] + + def test_create_rejects_invalid_name(self, client, isolated_profiles): + resp = client.post( + "/api/skills", + json={"name": "../escape", "content": SKILL_MD.format(name="x")}, + ) + assert resp.status_code == 400 + + +class TestSkillUpdate: + def test_update_rewrites_skill_md(self, client, isolated_profiles): + new_content = SKILL_MD.format(name="dashboard-skill").replace( + "Do the thing.", "Do the NEW thing." + ) + resp = client.put( + "/api/skills/content", + json={"name": "dashboard-skill", "content": new_content}, + ) + assert resp.status_code == 200 + skill_md = ( + isolated_profiles["default"] / "skills" / "dashboard-skill" / "SKILL.md" + ) + assert "Do the NEW thing." in skill_md.read_text(encoding="utf-8") + + def test_update_unknown_skill_404(self, client, isolated_profiles): + resp = client.put( + "/api/skills/content", + json={"name": "nope", "content": SKILL_MD.format(name="nope")}, + ) + assert resp.status_code == 404 + + def test_update_invalid_frontmatter_400(self, client, isolated_profiles): + resp = client.put( + "/api/skills/content", + json={"name": "dashboard-skill", "content": "broken"}, + ) + assert resp.status_code == 400 + + +class TestEditorEndpointsAuth: + @pytest.mark.parametrize( + "method,path,kwargs", + [ + ("get", "/api/skills/content?name=dashboard-skill", {}), + ("post", "/api/skills", {"json": {"name": "x", "content": "y"}}), + ("put", "/api/skills/content", {"json": {"name": "x", "content": "y"}}), + ], + ) + def test_endpoints_401_without_token( + self, client, isolated_profiles, method, path, kwargs + ): + from hermes_cli.web_server import _SESSION_HEADER_NAME + + client.headers.pop(_SESSION_HEADER_NAME, None) + resp = getattr(client, method)(path, **kwargs) + assert resp.status_code == 401 + + +class TestCronJobSkills: + def test_create_job_with_skills(self, client, isolated_profiles): + resp = client.post( + "/api/cron/jobs", + json={ + "prompt": "do work", + "schedule": "every 1h", + "name": "skilled-job", + "skills": ["dashboard-skill"], + }, + ) + assert resp.status_code == 200 + job = resp.json() + assert job["skills"] == ["dashboard-skill"] + + # Round-trip: the list endpoint carries the skills field too. + listed = client.get("/api/cron/jobs", params={"profile": "default"}).json() + match = [j for j in listed if j["id"] == job["id"]] + assert match and match[0]["skills"] == ["dashboard-skill"] + + def test_update_job_skills(self, client, isolated_profiles): + job = client.post( + "/api/cron/jobs", + json={"prompt": "do work", "schedule": "every 1h"}, + ).json() + assert job.get("skills") in (None, []) + + resp = client.put( + f"/api/cron/jobs/{job['id']}", + json={"updates": {"skills": ["dashboard-skill", "worker-skill"]}}, + params={"profile": "default"}, + ) + assert resp.status_code == 200 + assert resp.json()["skills"] == ["dashboard-skill", "worker-skill"] + + # Clearing works too. + resp = client.put( + f"/api/cron/jobs/{job['id']}", + json={"updates": {"skills": []}}, + params={"profile": "default"}, + ) + assert resp.status_code == 200 + assert resp.json()["skills"] == [] diff --git a/web/src/components/SkillEditorDialog.tsx b/web/src/components/SkillEditorDialog.tsx new file mode 100644 index 00000000000..9981fe79dc8 --- /dev/null +++ b/web/src/components/SkillEditorDialog.tsx @@ -0,0 +1,215 @@ +import { useEffect, useState } from "react"; +import { api } from "@/lib/api"; +import { Button } from "@nous-research/ui/ui/components/button"; +import { Input } from "@nous-research/ui/ui/components/input"; +import { Label } from "@nous-research/ui/ui/components/label"; +import { Spinner } from "@nous-research/ui/ui/components/spinner"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogHeader, + DialogTitle, +} from "@nous-research/ui/ui/components/dialog"; + +/* ------------------------------------------------------------------ */ +/* SkillEditorDialog — create or edit a SKILL.md from the dashboard */ +/* */ +/* Headless/VPS users have no editor besides this: the only other way */ +/* to author a custom skill is SSH + a terminal editor. Create mode */ +/* posts a brand-new skill (name + optional category + SKILL.md); */ +/* edit mode loads the existing SKILL.md raw text and rewrites it. */ +/* Validation (frontmatter, name, size) happens server-side via the */ +/* same path the agent's skill_manage tool uses, so errors come back */ +/* as actionable messages rendered inline. */ +/* ------------------------------------------------------------------ */ + +const CREATE_TEMPLATE = `--- +name: my-skill +description: One-line description of when to use this skill. +--- + +# My Skill + +Numbered steps, exact commands, and pitfalls go here. +`; + +export interface SkillEditorDialogProps { + open: boolean; + /** Skill name to edit, or null for create mode. */ + editName: string | null; + /** Profile to scope reads/writes to ("" = the dashboard's own profile). */ + profile?: string; + onClose: () => void; + /** Called after a successful save so the page can refresh its list. */ + onSaved: (name: string) => void; +} + +export function SkillEditorDialog({ + open, + editName, + profile, + onClose, + onSaved, +}: SkillEditorDialogProps) { + // The body is remounted via `key` every time the dialog opens or the + // target skill changes, so all form state initializes through useState + // initializers — no reset-on-open effect (react-hooks/set-state-in-effect). + return ( + !o && onClose()}> + + {open && ( + + )} + + + ); +} + +function EditorBody({ + editName, + profile, + onClose, + onSaved, +}: Omit) { + const isEdit = editName !== null; + const [name, setName] = useState(""); + const [category, setCategory] = useState(""); + const [content, setContent] = useState(isEdit ? "" : CREATE_TEMPLATE); + const [loading, setLoading] = useState(isEdit); + const [saving, setSaving] = useState(false); + const [error, setError] = useState(null); + + useEffect(() => { + if (!editName) return; + let cancelled = false; + api + .getSkillContent(editName, profile || undefined) + .then((res) => !cancelled && setContent(res.content)) + .catch((e) => !cancelled && setError(String(e))) + .finally(() => !cancelled && setLoading(false)); + return () => { + cancelled = true; + }; + }, [editName, profile]); + + const handleSave = async () => { + setError(null); + if (!isEdit && !name.trim()) { + setError("Skill name is required."); + return; + } + if (!content.trim()) { + setError("SKILL.md content is required."); + return; + } + setSaving(true); + try { + if (isEdit) { + await api.updateSkillContent(editName, content, profile || undefined); + onSaved(editName); + } else { + const trimmed = name.trim(); + await api.createSkill( + { + name: trimmed, + content, + category: category.trim() || undefined, + }, + profile || undefined, + ); + onSaved(trimmed); + } + onClose(); + } catch (e) { + setError(String(e)); + } finally { + setSaving(false); + } + }; + + return ( + <> + + + {isEdit ? `Edit skill: ${editName}` : "New skill"} + + + {isEdit + ? "Rewrite this skill's SKILL.md. Frontmatter (name, description) is validated on save." + : "Author a custom skill — YAML frontmatter plus markdown instructions. It becomes available to the agent and attachable to cron jobs."} + + + +
+ {!isEdit && ( +
+
+ + setName(e.target.value)} + /> +
+
+ + setCategory(e.target.value)} + /> +
+
+ )} + +
+ + {loading ? ( +
+ +
+ ) : ( +