From 3733e4aff52d19d110eb415a3809e1b5ae07384a Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 24 Aug 2026 20:02:52 -0700 Subject: [PATCH] fix: system prompt no longer references tools/skills the session can't use; hermes-agent skill is always kept MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit finding (Blank Slate): the system prompt advertised web_search, skill_view, todo, and the hermes-agent skill even when the toolset had none of them — the model chases phantoms it can't call. - hermes-agent skill is now essential: cannot be disabled (config reads strip it, hermes tools writes drop it), cannot be deleted by skill_manage, is re-seeded past curator suppression, and is seeded even on .no-bundled-skills profiles (Blank Slate / --no-skills). - Blank Slate core toolsets grow from file+terminal to file+terminal+vision+skills: read_file cannot read images and points at vision_analyze; the essential skill needs skill_view to load. - HERMES_AGENT_HELP_GUIDANCE degrades to a docs-URL-only variant when skill tools are absent. - Execution-discipline guidance drops its web_search lines when web tools are off (execution_guidance_text renderer). - Skills-index preamble says 'basic tools like terminal' instead of naming web_search when web tools are off. - Coding operating brief drops the todo-tracking sentence when the todo tool isn't loaded. All gating keys off agent.valid_tool_names, fixed at session construction — prompt stays byte-stable per session (cache-safe). --- agent/coding_context.py | 20 ++- agent/prompt_builder.py | 41 +++++- agent/skill_utils.py | 15 +- agent/system_prompt.py | 16 ++- hermes_cli/profiles.py | 12 +- hermes_cli/setup.py | 38 +++-- hermes_cli/skills_config.py | 17 ++- tests/agent/test_phantom_tool_references.py | 148 ++++++++++++++++++++ tests/agent/test_system_prompt.py | 1 + tests/hermes_cli/test_setup_blank_slate.py | 14 +- tools/skill_manager_tool.py | 16 ++- tools/skills_sync.py | 71 +++++++--- 12 files changed, 355 insertions(+), 54 deletions(-) create mode 100644 tests/agent/test_phantom_tool_references.py diff --git a/agent/coding_context.py b/agent/coding_context.py index aa38305e01..333de26b05 100644 --- a/agent/coding_context.py +++ b/agent/coding_context.py @@ -520,13 +520,21 @@ class RuntimeMode: return None return [self.profile.toolset, *_enabled_mcp_servers(config)] - def system_prompt_parts(self) -> tuple[list[str], list[str], list[str]]: + def system_prompt_parts( + self, valid_tool_names=None + ) -> tuple[list[str], list[str], list[str]]: """Return prefix, workspace, and trailing posture blocks separately. The operating brief carries a model-family edit-format nudge appended to it (one cached string, not a separate block) so the model is steered toward the `patch` mode it handles best — see ``_edit_format_line``. + ``valid_tool_names`` (when provided) tailors the brief to the session's + toolset: the ``todo`` tracking sentence is dropped when the todo tool + isn't loaded (e.g. Blank Slate), so the brief never references a tool + the model can't call. The toolset is fixed at session construction, + so the rendered brief is deterministic per session — cache-safe. + The three lists preserve the historical flat prompt order: the brief, the live workspace snapshot, then configured operator instructions. Prompt assembly can therefore put a cache boundary before the snapshot @@ -539,6 +547,13 @@ class RuntimeMode: trailing: list[str] = [] if self.profile.guidance: brief = self.profile.guidance + if valid_tool_names is not None and "todo" not in valid_tool_names: + brief = brief.replace( + "- Track multi-step work with `todo`. Reference code as " + "`path:line` instead of pasting whole files.", + "- Reference code as `path:line` instead of pasting " + "whole files.", + ) edit_line = _edit_format_line(self.model) if edit_line: brief = f"{brief}\n{edit_line}" @@ -666,11 +681,12 @@ def coding_system_prompt_parts( cwd: Optional[str | Path] = None, config: Optional[dict[str, Any]] = None, model: Optional[str] = None, + valid_tool_names=None, ) -> tuple[list[str], list[str], list[str]]: """Return coding prefix, workspace snapshot, and trailing guidance.""" return resolve_runtime_mode( platform=platform, cwd=cwd, config=config, model=model - ).system_prompt_parts() + ).system_prompt_parts(valid_tool_names=valid_tool_names) def coding_compact_skill_categories( diff --git a/agent/prompt_builder.py b/agent/prompt_builder.py index 0e9359ad88..8a17a53e7f 100644 --- a/agent/prompt_builder.py +++ b/agent/prompt_builder.py @@ -168,6 +168,20 @@ HERMES_AGENT_HELP_GUIDANCE = ( "of truth when the two differ." ) +# Variant injected when the skill tools are not in the session's toolset +# (e.g. a Blank Slate install with the skills toolset disabled). Pointing the +# model at skill_view() there would be a dangling reference — the docs URL is +# the only actionable pointer. +HERMES_AGENT_HELP_GUIDANCE_NO_SKILLS = ( + "You run on Hermes Agent (by Nous Research). When the user needs help with " + "Hermes itself — configuring, setting up, using, extending, or troubleshooting " + "it — or when you need to understand your own features, tools, or capabilities, " + "the documentation at https://hermes-agent.nousresearch.com/docs is the " + "authoritative reference and always holds the latest, most up-to-date " + "information. Point the user there (or read it yourself if you have a way to " + "fetch web content)." +) + MEMORY_GUIDANCE = ( "You have persistent memory across sessions. Save durable facts using the memory " "tool: user preferences, environment details, tool quirks, and stable conventions. " @@ -563,6 +577,26 @@ OPENAI_MODEL_EXECUTION_GUIDANCE = ( "" ) + +def execution_guidance_text(valid_tool_names=None) -> str: + """Render OPENAI_MODEL_EXECUTION_GUIDANCE for the session's toolset. + + The block names ``web_search`` as the lookup tool for current facts; on + sessions without web tools (e.g. Blank Slate) that's a dangling + reference, so the web_search lines are dropped/adjusted. Deterministic + per-session (toolset is fixed at construction), so cache-safe. + """ + text = OPENAI_MODEL_EXECUTION_GUIDANCE + if valid_tool_names is not None and "web_search" not in valid_tool_names: + text = text.replace( + "- Current facts (weather, news, versions) → use web_search\n", "" + ) + text = text.replace( + "(search_files, web_search, read_file, etc.)", + "(search_files, read_file, etc.)", + ) + return text + # Gemini/Gemma-specific operational guidance, adapted from OpenCode's gemini.txt. # Injected alongside TOOL_USE_ENFORCEMENT_GUIDANCE when the model is Gemini or Gemma. GOOGLE_MODEL_OPERATIONAL_GUIDANCE = ( @@ -2177,6 +2211,11 @@ def _build_skills_system_prompt_inner( if not skills_by_category: result = "" else: + # "basic tools like web_search or terminal" — don't name web_search + # when the session has no web tools (dangling reference otherwise). + _basic_tools = "web_search or terminal" + if available_tools is not None and "web_search" not in available_tools: + _basic_tools = "terminal" index_lines = [] for category in sorted(skills_by_category.keys()): # Deduplicate and sort skills within each category @@ -2207,7 +2246,7 @@ def _build_skills_system_prompt_inner( "than to miss critical steps, pitfalls, or established workflows. " "Skills contain specialized knowledge — API endpoints, tool-specific commands, " "and proven workflows that outperform general-purpose approaches. Load the skill " - "even if you think you could handle the task with basic tools like web_search or terminal. " + f"even if you think you could handle the task with basic tools like {_basic_tools}. " "Skills also encode the user's preferred approach, conventions, and quality standards " "for tasks like code review, planning, and testing — load them even for tasks you " "already know how to do, because the skill defines how it should be done here.\n" diff --git a/agent/skill_utils.py b/agent/skill_utils.py index 01ad461879..0c893ab465 100644 --- a/agent/skill_utils.py +++ b/agent/skill_utils.py @@ -434,6 +434,15 @@ def _load_raw_config() -> Dict[str, Any]: return parsed +# Skills that must stay available regardless of configuration. The +# `hermes-agent` skill is the agent's own operating manual — it drives +# configuring, extending, and troubleshooting Hermes itself, and the system +# prompt unconditionally points at it. Disabling it leaves the agent unable +# to help with Hermes, so disable requests for these names are ignored +# everywhere the disabled list is consulted. +ESSENTIAL_SKILLS: frozenset = frozenset({"hermes-agent"}) + + def get_disabled_skill_names(platform: str | None = None) -> Set[str]: """Read disabled skill names from config.yaml. @@ -468,8 +477,10 @@ def get_disabled_skill_names(platform: str | None = None) -> Set[str]: resolved_platform ) if platform_disabled is not None: - return global_disabled | _normalize_string_set(platform_disabled) - return global_disabled + return ( + global_disabled | _normalize_string_set(platform_disabled) + ) - ESSENTIAL_SKILLS + return global_disabled - ESSENTIAL_SKILLS def parse_config_string_list(value) -> List[str]: diff --git a/agent/system_prompt.py b/agent/system_prompt.py index b176a2554f..50d088630e 100644 --- a/agent/system_prompt.py +++ b/agent/system_prompt.py @@ -36,6 +36,7 @@ from agent.prompt_builder import ( EXECUTION_GUIDANCE_MODELS, GOOGLE_MODEL_OPERATIONAL_GUIDANCE, HERMES_AGENT_HELP_GUIDANCE, + HERMES_AGENT_HELP_GUIDANCE_NO_SKILLS, KANBAN_GUIDANCE, MEMORY_GUIDANCE, USER_PROFILE_GUIDANCE, @@ -391,8 +392,15 @@ def build_system_prompt_parts(agent: Any, system_message: Optional[str] = None) # Fallback to hardcoded identity stable_parts.append(DEFAULT_AGENT_IDENTITY) - # Pointer to the hermes-agent skill + docs for user questions about Hermes itself. - stable_parts.append(HERMES_AGENT_HELP_GUIDANCE) + # Pointer to the hermes-agent skill + docs for user questions about Hermes + # itself. When the session has no skill tools (Blank Slate with the skills + # toolset off), skill_view() would be a dangling reference — inject the + # docs-only variant instead. Toolset is fixed per-session, so cache-safe. + _has_skill_view = "skill_view" in (agent.valid_tool_names or set()) + stable_parts.append( + HERMES_AGENT_HELP_GUIDANCE if _has_skill_view + else HERMES_AGENT_HELP_GUIDANCE_NO_SKILLS + ) # Universal task-completion / no-fabrication guidance. Applied to ALL # models regardless of tool_use_enforcement gating — the failure modes @@ -521,7 +529,8 @@ def build_system_prompt_parts(agent: Any, system_message: Optional[str] = None) model_lower = (agent.model or "").lower() _exec_inject = any(p in model_lower for p in EXECUTION_GUIDANCE_MODELS) if _exec_inject: - stable_parts.append(OPENAI_MODEL_EXECUTION_GUIDANCE) + from agent.prompt_builder import execution_guidance_text + stable_parts.append(execution_guidance_text(agent.valid_tool_names)) has_skills_tools = any(name in agent.valid_tool_names for name in ['skills_list', 'skill_view', 'skill_manage']) if has_skills_tools: @@ -590,6 +599,7 @@ def build_system_prompt_parts(agent: Any, system_message: Optional[str] = None) platform=agent.platform, cwd=resolve_context_cwd(), model=agent.model, + valid_tool_names=agent.valid_tool_names, ) stable_parts.extend(coding_prefix_parts) except Exception: diff --git a/hermes_cli/profiles.py b/hermes_cli/profiles.py index fbcd9cbd87..d5a4fa4e29 100644 --- a/hermes_cli/profiles.py +++ b/hermes_cli/profiles.py @@ -1324,16 +1324,10 @@ def seed_profile_skills(profile_dir: Path, quiet: bool = False) -> Optional[dict Profiles that opted out of bundled skills (via ``hermes profile create --no-skills`` — which writes ``.no-bundled-skills`` to the profile root) - are skipped and get an empty-result dict so callers can report - "opted out" instead of "failed". + still run the sync: ``sync_skills()`` detects the marker itself and seeds + only the essential skills (e.g. ``hermes-agent``), reporting + ``skipped_opt_out`` so callers can say "opted out" instead of "failed". """ - if has_bundled_skills_opt_out(profile_dir): - return { - "copied": [], - "updated": [], - "user_modified": [], - "skipped_opt_out": True, - } project_root = Path(__file__).parent.parent.resolve() try: result = subprocess.run( diff --git a/hermes_cli/setup.py b/hermes_cli/setup.py index 7a959bf7fe..4f0d190203 100644 --- a/hermes_cli/setup.py +++ b/hermes_cli/setup.py @@ -3433,20 +3433,28 @@ def _run_first_time_quick_setup(config: dict, hermes_home, is_existing: bool): def _blank_slate_minimal_toolsets(config: dict): """Write the minimal toolset state for a Blank Slate install. - Only ``file`` and ``terminal`` are enabled. Two layers enforce this: + Only ``file``, ``terminal``, ``vision``, and ``skills`` are enabled. + Vision is part of + the core surface: ``read_file`` cannot read images and its own description + points at ``vision_analyze``, so an agent without it can't see screenshots + or image files at all. Skills stay on because the essential + ``hermes-agent`` skill (the agent's operating manual for driving, + configuring, and troubleshooting Hermes) is always seeded — without + ``skill_view`` it would be unloadable. Two layers enforce the selection: - 1. ``platform_toolsets["cli"] = ["file", "terminal"]`` — an explicit list of + 1. ``platform_toolsets["cli"] = ["file", "skills", "terminal", "vision"]`` + — an explicit list of configurable keys, which the resolver treats as authoritative (``has_explicit_config``) so default toolsets aren't re-expanded. 2. ``agent.disabled_toolsets`` — a global hard-suppression list (applied last in ``_get_platform_tools``, overriding every other path including the non-configurable platform-toolset recovery that would otherwise re-add - toolsets like ``kanban``). We list every known toolset except the two we + toolsets like ``kanban``). We list every known toolset except the ones we keep, guaranteeing a true blank slate regardless of platform/recovery quirks. The user re-enables any of them later via ``hermes tools`` (which rewrites ``platform_toolsets``) or by editing ``agent.disabled_toolsets``. """ - keep = {"file", "terminal"} + keep = {"file", "terminal", "vision", "skills"} config.setdefault("platform_toolsets", {})["cli"] = sorted(keep) try: @@ -3523,9 +3531,11 @@ def _run_blank_slate_setup(config: dict, hermes_home, is_existing: bool): print_info("to run an agent, then you choose whether to stop there or walk") print_info("through enabling more — opting in to exactly what you want.") print_info("") - print_info("Forced on: Provider & Model, File Operations, Terminal.") - print_info("Everything else (web, browser, code exec, vision, memory,") - print_info("delegation, cron, skills, plugins, MCP, …) starts disabled.") + print_info("Forced on: Provider & Model, File Operations, Terminal, Vision, Skills.") + print_info("Everything else (web, browser, code exec, memory,") + print_info("delegation, cron, plugins, MCP, …) starts disabled. The") + print_info("essential `hermes-agent` skill is always kept so the agent") + print_info("can help you drive and configure Hermes itself.") print() # ── Step 1: Provider & Model (REQUIRED — the agent cannot run without it) ── @@ -3543,7 +3553,7 @@ def _run_blank_slate_setup(config: dict, hermes_home, is_existing: bool): save_config(config) print() print_success("Minimal baseline applied:") - print_info(" Toolsets: file, terminal (everything else off)") + print_info(" Toolsets: file, terminal, vision, skills (everything else off)") print_info(" Compression, memory, checkpoints, smart routing: off") # ── The fork: stop here, or walk through enabling things ── @@ -3561,10 +3571,12 @@ def _run_blank_slate_setup(config: dict, hermes_home, is_existing: bool): if path == 0: save_config(config) # Blank Slate means no bundled skills; record the opt-out so future - # `hermes update` runs don't re-inject them. + # `hermes update` runs don't re-inject them. Essential skills (the + # `hermes-agent` operating manual) are still seeded by the sync. try: - from tools.skills_sync import set_bundled_skills_opt_out + from tools.skills_sync import set_bundled_skills_opt_out, sync_skills set_bundled_skills_opt_out(True) + sync_skills(quiet=True) except Exception as exc: logger.debug("blank-slate skill opt-out error: %s", exc) print() @@ -3605,7 +3617,11 @@ def _blank_slate_walkthrough(config: dict, hermes_home): print_success(f"Seeded {copied} bundled skills.") else: set_bundled_skills_opt_out(True) - print_info("No skills seeded. A .no-bundled-skills marker keeps future") + # Essential skills (the `hermes-agent` operating manual) are + # still seeded even for an opted-out profile. + sync_skills(quiet=True) + print_info("No skills seeded (except the essential `hermes-agent`") + print_info("skill). A .no-bundled-skills marker keeps future") print_info("`hermes update` runs from re-injecting them. Opt back in any") print_info("time with `hermes skills opt-in --sync`.") except Exception as exc: diff --git a/hermes_cli/skills_config.py b/hermes_cli/skills_config.py index 720b9eb3f0..4a19e052a6 100644 --- a/hermes_cli/skills_config.py +++ b/hermes_cli/skills_config.py @@ -52,17 +52,26 @@ def get_disabled_skills(config: dict, platform: Optional[str] = None) -> Set[str skills_cfg = config.get("skills") or {} if not isinstance(skills_cfg, dict): return set() + from agent.skill_utils import ESSENTIAL_SKILLS global_disabled = _normalize_skill_names(skills_cfg.get("disabled")) if platform is None: - return global_disabled + return global_disabled - ESSENTIAL_SKILLS platform_disabled = cfg_get(skills_cfg, "platform_disabled", platform) if platform_disabled is None: - return global_disabled - return global_disabled | _normalize_skill_names(platform_disabled) + return global_disabled - ESSENTIAL_SKILLS + return ( + global_disabled | _normalize_skill_names(platform_disabled) + ) - ESSENTIAL_SKILLS def save_disabled_skills(config: dict, disabled: Set[str], platform: Optional[str] = None): - """Persist disabled skill names to config.""" + """Persist disabled skill names to config. + + Essential skills (e.g. ``hermes-agent``) are silently dropped from the + list — they cannot be disabled from any surface. + """ + from agent.skill_utils import ESSENTIAL_SKILLS + disabled = set(disabled) - ESSENTIAL_SKILLS config.setdefault("skills", {}) if platform is None: config["skills"]["disabled"] = sorted(disabled) diff --git a/tests/agent/test_phantom_tool_references.py b/tests/agent/test_phantom_tool_references.py new file mode 100644 index 0000000000..045f356198 --- /dev/null +++ b/tests/agent/test_phantom_tool_references.py @@ -0,0 +1,148 @@ +"""Phantom tool references: system-prompt blocks must not name tools the +session can't call (Blank Slate audit, Aug 2026). + +Covers: + * HERMES_AGENT_HELP_GUIDANCE degrades to the docs-only variant when the + skill tools aren't loaded. + * execution_guidance_text() drops web_search lines when web tools are off. + * The coding operating brief drops the `todo` sentence when the todo tool + isn't loaded. + * ESSENTIAL_SKILLS can't be disabled via config, and the CLI writer strips + them from persisted disabled lists. +""" + +from pathlib import Path + + +class TestHermesAgentHelpGuidance: + def test_skill_variant_used_when_skill_view_present(self): + from agent.prompt_builder import HERMES_AGENT_HELP_GUIDANCE + assert "skill_view(name='hermes-agent')" in HERMES_AGENT_HELP_GUIDANCE + + def test_no_skills_variant_has_no_skill_view_reference(self): + from agent.prompt_builder import HERMES_AGENT_HELP_GUIDANCE_NO_SKILLS + assert "skill_view" not in HERMES_AGENT_HELP_GUIDANCE_NO_SKILLS + assert "hermes-agent.nousresearch.com/docs" in HERMES_AGENT_HELP_GUIDANCE_NO_SKILLS + + +class TestExecutionGuidanceText: + def test_full_text_when_web_search_available(self): + from agent.prompt_builder import ( + OPENAI_MODEL_EXECUTION_GUIDANCE, + execution_guidance_text, + ) + assert execution_guidance_text({"web_search", "terminal"}) == ( + OPENAI_MODEL_EXECUTION_GUIDANCE + ) + + def test_full_text_when_toolset_unknown(self): + from agent.prompt_builder import ( + OPENAI_MODEL_EXECUTION_GUIDANCE, + execution_guidance_text, + ) + assert execution_guidance_text(None) == OPENAI_MODEL_EXECUTION_GUIDANCE + + def test_web_search_dropped_without_web_tools(self): + from agent.prompt_builder import execution_guidance_text + text = execution_guidance_text({"terminal", "read_file"}) + assert "web_search" not in text + # The surrounding structure survives. + assert "" in text + assert "" in text + assert "(search_files, read_file, etc.)" in text + + +class TestCodingBriefTodoGating: + def _brief(self, valid_tool_names): + from agent.coding_context import CODING_PROFILE, RuntimeMode + mode = RuntimeMode( + profile=CODING_PROFILE, surface="cli", cwd=Path.cwd(), + ) + prefix, _ws, _tr = mode.system_prompt_parts( + valid_tool_names=valid_tool_names + ) + assert prefix, "coding profile must emit an operating brief" + return prefix[0] + + def test_todo_kept_when_tool_available(self): + brief = self._brief({"todo", "terminal", "read_file"}) + assert "Track multi-step work with `todo`" in brief + + def test_todo_dropped_when_tool_missing(self): + brief = self._brief({"terminal", "read_file"}) + assert "`todo`" not in brief + # The path:line half of the merged bullet survives. + assert "path:line" in brief + + def test_unknown_toolset_keeps_full_brief(self): + brief = self._brief(None) + assert "Track multi-step work with `todo`" in brief + + +class TestEssentialSkillsUndisableable: + def test_agent_side_reader_strips_essential(self, monkeypatch, tmp_path): + import agent.skill_utils as su + cfg = tmp_path / "config.yaml" + cfg.write_text( + "skills:\n disabled:\n - hermes-agent\n - some-other-skill\n", + encoding="utf-8", + ) + monkeypatch.setattr(su, "get_config_path", lambda: cfg) + su._RAW_CONFIG_CACHE.clear() + disabled = su.get_disabled_skill_names(platform="cli") + assert "hermes-agent" not in disabled + assert "some-other-skill" in disabled + + def test_cli_side_reader_strips_essential(self): + from hermes_cli.skills_config import get_disabled_skills + cfg = {"skills": {"disabled": ["hermes-agent", "other"]}} + disabled = get_disabled_skills(cfg) + assert "hermes-agent" not in disabled + assert "other" in disabled + + def test_cli_side_writer_strips_essential(self, monkeypatch): + import hermes_cli.skills_config as sc + saved = {} + monkeypatch.setattr(sc, "save_config", lambda cfg: saved.update(cfg)) + cfg = {} + sc.save_disabled_skills(cfg, {"hermes-agent", "other"}) + assert cfg["skills"]["disabled"] == ["other"] + + def test_skill_manage_delete_refused(self): + from tools.skill_manager_tool import _pinned_guard + msg = _pinned_guard("hermes-agent") + assert msg is not None + assert "essential" in msg.lower() + + +class TestEssentialOnlySync: + def test_opted_out_sync_seeds_only_essential(self, monkeypatch, tmp_path): + """A profile with .no-bundled-skills still gets the hermes-agent skill.""" + import tools.skills_sync as ss + + home = tmp_path / ".hermes" + home.mkdir() + (home / ss.NO_BUNDLED_SKILLS_MARKER).write_text("", encoding="utf-8") + + bundled = tmp_path / "bundled" + for cat, name in [ + ("autonomous-ai-agents", "hermes-agent"), + ("media", "gif-search"), + ]: + d = bundled / cat / name + d.mkdir(parents=True) + (d / "SKILL.md").write_text( + f"---\nname: {name}\ndescription: x\n---\nbody\n", + encoding="utf-8", + ) + + monkeypatch.setattr(ss, "_hermes_home", lambda: home) + monkeypatch.setattr(ss, "_get_bundled_dir", lambda: bundled) + monkeypatch.setattr(ss, "_build_external_skill_index", lambda: set()) + + result = ss.sync_skills(quiet=True) + + assert result["skipped_opt_out"] is True + assert result["copied"] == ["hermes-agent"] + assert (home / "skills" / "autonomous-ai-agents" / "hermes-agent" / "SKILL.md").exists() + assert not (home / "skills" / "media").exists() diff --git a/tests/agent/test_system_prompt.py b/tests/agent/test_system_prompt.py index 28d0e6d3bf..deb0ee1651 100644 --- a/tests/agent/test_system_prompt.py +++ b/tests/agent/test_system_prompt.py @@ -290,6 +290,7 @@ def test_coding_prompt_preserves_legacy_workspace_order(monkeypatch): ) monkeypatch.setattr(system_prompt, "DEFAULT_AGENT_IDENTITY", "IDENTITY") monkeypatch.setattr(system_prompt, "HERMES_AGENT_HELP_GUIDANCE", "HELP") + monkeypatch.setattr(system_prompt, "HERMES_AGENT_HELP_GUIDANCE_NO_SKILLS", "HELP") monkeypatch.setattr(system_prompt, "STEER_CHANNEL_NOTE", "STEER") monkeypatch.setattr(system_prompt, "get_hermes_home", lambda: Path("/hermes")) diff --git a/tests/hermes_cli/test_setup_blank_slate.py b/tests/hermes_cli/test_setup_blank_slate.py index 5289322c53..b401a2069e 100644 --- a/tests/hermes_cli/test_setup_blank_slate.py +++ b/tests/hermes_cli/test_setup_blank_slate.py @@ -39,13 +39,20 @@ class TestBlankSlateMinimalToolsets: - def test_tool_schema_survives_disabled_toolsets_from_config(self): + def test_tool_schema_survives_disabled_toolsets_from_config(self, monkeypatch): """Regression: disabled_toolsets must not erase the minimal Blank Slate surface when passed to model_tools. Before the fix, posture toolsets like ``coding`` in disabled_toolsets caused model_tools to subtract terminal, read_file, write_file, etc. (#57315). + + vision_analyze is additionally check_fn-gated on a resolvable vision + backend; mock the requirement check so the toolset logic is exercised + independent of the test host's provider credentials. """ import model_tools + from tools.registry import registry as _tool_registry + _entry = _tool_registry.get_entry("vision_analyze") + monkeypatch.setattr(_entry, "check_fn", lambda: True) from hermes_cli.tools_config import _get_platform_tools cfg = {} _blank_slate_minimal_toolsets(cfg) @@ -61,7 +68,8 @@ class TestBlankSlateMinimalToolsets: {(d.get("function") or {}).get("name") or d.get("name") for d in defs} ) assert names == ["patch", "process", "read_file", "search_files", - "terminal", "write_file"] + "skill_manage", "skill_view", "skills_list", + "terminal", "vision_analyze", "write_file"] class TestBlankSlateMinimizeConfig: @@ -107,7 +115,7 @@ class TestBlankSlateFork: s._run_blank_slate_setup(cfg, tmp_path, is_existing=False) # Minimal baseline was applied, walkthrough was NOT run. - assert cfg["platform_toolsets"]["cli"] == ["file", "terminal"] + assert cfg["platform_toolsets"]["cli"] == ["file", "skills", "terminal", "vision"] assert walked["called"] is False # Finish-now path records the skill opt-out (no bundled skills). assert opted_out["value"] is True diff --git a/tools/skill_manager_tool.py b/tools/skill_manager_tool.py index b444056747..fa462ec898 100644 --- a/tools/skill_manager_tool.py +++ b/tools/skill_manager_tool.py @@ -272,16 +272,30 @@ def _validate_delete_target(skill_dir: Path) -> Optional[str]: def _pinned_guard(name: str) -> Optional[str]: - """Return a refusal message if *name* is pinned, else None. + """Return a refusal message if *name* is pinned or essential, else None. Pin protects a skill from **deletion** — both the curator's auto-archive passes and the agent's ``skill_manage(action="delete")`` tool call. The agent can still patch/edit pinned skills; pin only guards against irrecoverable loss, not against content evolution. + Essential skills (``agent/skill_utils.ESSENTIAL_SKILLS``, e.g. + ``hermes-agent``) are treated as permanently pinned: the system prompt + always references them, so deleting one leaves a dangling instruction. + Best-effort: if the sidecar is unreadable we let the delete through rather than block on a broken telemetry file. """ + try: + from agent.skill_utils import ESSENTIAL_SKILLS + if name in ESSENTIAL_SKILLS: + return ( + f"Skill '{name}' is essential to Hermes (the agent's own " + f"operating manual referenced by the system prompt) and " + f"cannot be deleted. Patches and edits are still allowed." + ) + except Exception: + logger.debug("essential-guard lookup failed for %s", name, exc_info=True) try: from tools import skill_usage rec = skill_usage.get_record(name) diff --git a/tools/skills_sync.py b/tools/skills_sync.py index 14bfbc7e4d..cdf3ab0925 100644 --- a/tools/skills_sync.py +++ b/tools/skills_sync.py @@ -105,6 +105,15 @@ def _manifest_file() -> Path: NO_BUNDLED_SKILLS_MARKER = ".no-bundled-skills" +def _essential_names() -> frozenset: + """Names of skills that must always exist (see skill_utils.ESSENTIAL_SKILLS).""" + try: + from agent.skill_utils import ESSENTIAL_SKILLS + return ESSENTIAL_SKILLS + except Exception: + return frozenset({"hermes-agent"}) + + def _get_bundled_dir() -> Path: """Locate the bundled skills/ directory. @@ -708,18 +717,20 @@ def sync_skills(quiet: bool = False) -> dict: user_modified (list), cleaned (list), total_bundled (int) """ # Opt-out: a profile (named or the default ~/.hermes) that wrote the - # .no-bundled-skills marker gets zero bundled-skill seeding. Returning the - # empty-result shape with skipped_opt_out lets callers report "opted out" - # instead of "synced 0 / failed". This is the default-profile counterpart - # to seed_profile_skills()'s marker check for named profiles. - if (_hermes_home() / NO_BUNDLED_SKILLS_MARKER).exists(): - if not quiet: - print(" (skipped — profile opted out of bundled skills via .no-bundled-skills)") - return { - "copied": [], "updated": [], "skipped": 0, - "user_modified": [], "cleaned": [], "total_bundled": 0, - "optional_provenance_backfilled": [], "skipped_opt_out": True, - } + # .no-bundled-skills marker gets zero bundled-skill seeding — EXCEPT the + # essential skills (agent/skill_utils.ESSENTIAL_SKILLS). The + # ``hermes-agent`` skill is the agent's own operating manual and the + # system prompt always points at it, so even a Blank Slate / --no-skills + # profile keeps that one skill. Returning the empty-result shape with + # skipped_opt_out lets callers report "opted out" instead of + # "synced 0 / failed". This is the default-profile counterpart to + # seed_profile_skills()'s marker check for named profiles. + essential_only = (_hermes_home() / NO_BUNDLED_SKILLS_MARKER).exists() + if essential_only and not quiet: + print( + " (profile opted out of bundled skills via .no-bundled-skills — " + "seeding essential skills only)" + ) bundled_dir = _get_bundled_dir() if not bundled_dir.exists(): @@ -732,6 +743,12 @@ def sync_skills(quiet: bool = False) -> dict: _skills_dir().mkdir(parents=True, exist_ok=True) manifest = _read_manifest() bundled_skills = _discover_bundled_skills(bundled_dir) + if essential_only: + # Opted-out profile: only the essential skills are synced. + bundled_skills = [ + (name, src) for name, src in bundled_skills + if name in _essential_names() + ] bundled_names = {name for name, _ in bundled_skills} suppressed = _read_suppressed_names() # Index of skills already provided by external_dirs (skip writing them) @@ -755,7 +772,8 @@ def sync_skills(quiet: bool = False) -> dict: # archives a bundled skill with curator.prune_builtins enabled. Without # this skip, every `hermes update` would resurrect a skill the user # deliberately pruned. Restoring the skill clears its suppression entry. - if skill_name in suppressed: + # Essential skills are exempt — they must always come back. + if skill_name in suppressed and skill_name not in _essential_names(): suppressed_skipped.append(skill_name) continue @@ -942,15 +960,29 @@ def sync_skills(quiet: bool = False) -> dict: # ── In manifest but not on disk — user deleted it ── skipped += 1 - # Clean stale manifest entries (skills removed from bundled dir) - cleaned = sorted(set(manifest.keys()) - bundled_names) - for name in cleaned: - del manifest[name] + # Clean stale manifest entries (skills removed from bundled dir). + # Skip on an opted-out profile: bundled_skills was filtered to the + # essential set there, and cleaning would drop tracking for every other + # previously-synced skill still on disk. + if essential_only: + cleaned = [] + else: + cleaned = sorted(set(manifest.keys()) - bundled_names) + for name in cleaned: + del manifest[name] - # Also copy DESCRIPTION.md files for categories (if not already present) + # Also copy DESCRIPTION.md files for categories (if not already present). + # On an opted-out profile only the essential skills' own category + # descriptions are seeded — not the full catalog's. + _essential_cat_dirs = { + _compute_relative_dest(src, bundled_dir).parent + for _, src in bundled_skills + } if essential_only else None for desc_md in bundled_dir.rglob("DESCRIPTION.md"): rel = desc_md.relative_to(bundled_dir) dest_desc = _skills_dir() / rel + if _essential_cat_dirs is not None and dest_desc.parent not in _essential_cat_dirs: + continue if not dest_desc.exists(): try: dest_desc.parent.mkdir(parents=True, exist_ok=True) @@ -972,6 +1004,9 @@ def sync_skills(quiet: bool = False) -> dict: "total_bundled": len(bundled_skills), "optional_provenance_backfilled": optional_provenance_backfilled, "shadowed_by_external": shadowed_by_external, + # Opted-out profiles still seed essential skills; the flag lets + # callers report "opted out" rather than a normal full sync. + "skipped_opt_out": essential_only, }