mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
refactor(commands): drop /provider, /plan handler, and clean up slash registry (#15047)
* refactor(commands): drop /provider and clean up slash registry * refactor(commands): drop /plan special handler — use plain skill dispatch
This commit is contained in:
Teknium
GitHub
parent
b29287258a
commit
b2e124d082
20 changed files with 21 additions and 535 deletions
|
|
@ -1,13 +1,11 @@
|
|||
"""Tests for agent/skill_commands.py — skill slash command scanning and platform filtering."""
|
||||
|
||||
import os
|
||||
from datetime import datetime
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import tools.skills_tool as skills_tool_module
|
||||
from agent.skill_commands import (
|
||||
build_plan_path,
|
||||
build_preloaded_skills_prompt,
|
||||
build_skill_invocation_message,
|
||||
resolve_skill_command_key,
|
||||
|
|
@ -399,40 +397,6 @@ Generate some audio.
|
|||
assert 'file_path="<path>"' in msg
|
||||
|
||||
|
||||
class TestPlanSkillHelpers:
|
||||
def test_build_plan_path_uses_workspace_relative_dir_and_slugifies_request(self):
|
||||
path = build_plan_path(
|
||||
"Implement OAuth login + refresh tokens!",
|
||||
now=datetime(2026, 3, 15, 9, 30, 45),
|
||||
)
|
||||
|
||||
assert path == Path(".hermes") / "plans" / "2026-03-15_093045-implement-oauth-login-refresh-tokens.md"
|
||||
|
||||
def test_plan_skill_message_can_include_runtime_save_path_note(self, tmp_path):
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
_make_skill(
|
||||
tmp_path,
|
||||
"plan",
|
||||
body="Save plans under .hermes/plans in the active workspace and do not execute the work.",
|
||||
)
|
||||
scan_skill_commands()
|
||||
msg = build_skill_invocation_message(
|
||||
"/plan",
|
||||
"Add a /plan command",
|
||||
runtime_note=(
|
||||
"Save the markdown plan with write_file to this exact relative path inside "
|
||||
"the active workspace/backend cwd: .hermes/plans/plan.md"
|
||||
),
|
||||
)
|
||||
|
||||
assert msg is not None
|
||||
assert "Save plans under $HERMES_HOME/plans" not in msg
|
||||
assert ".hermes/plans" in msg
|
||||
assert "Add a /plan command" in msg
|
||||
assert ".hermes/plans/plan.md" in msg
|
||||
assert "Runtime note:" in msg
|
||||
|
||||
|
||||
class TestSkillDirectoryHeader:
|
||||
"""The activation message must expose the absolute skill directory and
|
||||
explain how to resolve relative paths, so skills with bundled scripts
|
||||
|
|
|
|||
|
|
@ -1,67 +0,0 @@
|
|||
"""Tests for the /plan CLI slash command."""
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from agent.skill_commands import scan_skill_commands
|
||||
from cli import HermesCLI
|
||||
|
||||
|
||||
def _make_cli():
|
||||
cli_obj = HermesCLI.__new__(HermesCLI)
|
||||
cli_obj.config = {}
|
||||
cli_obj.console = MagicMock()
|
||||
cli_obj.agent = None
|
||||
cli_obj.conversation_history = []
|
||||
cli_obj.session_id = "sess-123"
|
||||
cli_obj._pending_input = MagicMock()
|
||||
return cli_obj
|
||||
|
||||
|
||||
def _make_plan_skill(skills_dir):
|
||||
skill_dir = skills_dir / "plan"
|
||||
skill_dir.mkdir(parents=True, exist_ok=True)
|
||||
(skill_dir / "SKILL.md").write_text(
|
||||
"""---
|
||||
name: plan
|
||||
description: Plan mode skill.
|
||||
---
|
||||
|
||||
# Plan
|
||||
|
||||
Use the current conversation context when no explicit instruction is provided.
|
||||
Save plans under the active workspace's .hermes/plans directory.
|
||||
"""
|
||||
)
|
||||
|
||||
|
||||
class TestCLIPlanCommand:
|
||||
def test_plan_command_queues_plan_skill_message(self, tmp_path, monkeypatch):
|
||||
cli_obj = _make_cli()
|
||||
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
_make_plan_skill(tmp_path)
|
||||
scan_skill_commands()
|
||||
result = cli_obj.process_command("/plan Add OAuth login")
|
||||
|
||||
assert result is True
|
||||
cli_obj._pending_input.put.assert_called_once()
|
||||
queued = cli_obj._pending_input.put.call_args[0][0]
|
||||
assert "Plan mode skill" in queued
|
||||
assert "Add OAuth login" in queued
|
||||
assert ".hermes/plans" in queued
|
||||
assert str(tmp_path / "plans") not in queued
|
||||
assert "active workspace/backend cwd" in queued
|
||||
assert "Runtime note:" in queued
|
||||
|
||||
def test_plan_without_args_uses_skill_context_guidance(self, tmp_path, monkeypatch):
|
||||
cli_obj = _make_cli()
|
||||
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
_make_plan_skill(tmp_path)
|
||||
scan_skill_commands()
|
||||
cli_obj.process_command("/plan")
|
||||
|
||||
queued = cli_obj._pending_input.put.call_args[0][0]
|
||||
assert "current conversation context" in queued
|
||||
assert ".hermes/plans/" in queued
|
||||
assert "conversation-plan.md" in queued
|
||||
|
|
@ -73,14 +73,6 @@ class TestSlashCommands:
|
|||
send_status = await send_and_capture(adapter, "/status", platform)
|
||||
send_status.assert_called_once()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_provider_shows_current_provider(self, adapter, platform):
|
||||
send = await send_and_capture(adapter, "/provider", platform)
|
||||
|
||||
send.assert_called_once()
|
||||
response_text = send.call_args[1].get("content") or send.call_args[0][1]
|
||||
assert "provider" in response_text.lower()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_verbose_responds(self, adapter, platform):
|
||||
send = await send_and_capture(adapter, "/verbose", platform)
|
||||
|
|
|
|||
|
|
@ -272,7 +272,7 @@ class TestCommandBypassActiveSession:
|
|||
# Tests: non-bypass-set commands (no dedicated Level-2 handler) also bypass
|
||||
# instead of interrupting + being discarded. Regression for the Discord
|
||||
# ghost-slash-command bug where /model, /reasoning, /voice, /insights, /title,
|
||||
# /resume, /retry, /undo, /compress, /usage, /provider, /reload-mcp,
|
||||
# /resume, /retry, /undo, /compress, /usage, /reload-mcp,
|
||||
# /sethome, /reset silently interrupted the running agent.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
|
@ -298,7 +298,6 @@ class TestAllResolvableCommandsBypassGuard:
|
|||
("/undo", "undo"),
|
||||
("/compress", "compress"),
|
||||
("/usage", "usage"),
|
||||
("/provider", "provider"),
|
||||
("/reload-mcp", "reload-mcp"),
|
||||
("/sethome", "sethome"),
|
||||
],
|
||||
|
|
@ -326,7 +325,7 @@ class TestAllResolvableCommandsBypassGuard:
|
|||
|
||||
for cmd in (
|
||||
"model", "reasoning", "personality", "voice", "insights", "title",
|
||||
"resume", "retry", "undo", "compress", "usage", "provider",
|
||||
"resume", "retry", "undo", "compress", "usage",
|
||||
"reload-mcp", "sethome", "reset",
|
||||
):
|
||||
assert should_bypass_active_session(cmd) is True, (
|
||||
|
|
|
|||
|
|
@ -164,7 +164,7 @@ async def test_auto_registers_missing_gateway_commands(adapter):
|
|||
|
||||
# These commands are gateway-available but were not in the original
|
||||
# hardcoded registration list — they should be auto-registered.
|
||||
expected_auto = {"debug", "yolo", "reload", "profile"}
|
||||
expected_auto = {"debug", "yolo", "profile"}
|
||||
for name in expected_auto:
|
||||
assert name in tree_names, f"/{name} should be auto-registered on Discord"
|
||||
|
||||
|
|
|
|||
|
|
@ -1,129 +0,0 @@
|
|||
"""Tests for the /plan gateway slash command."""
|
||||
|
||||
from datetime import datetime
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.skill_commands import scan_skill_commands
|
||||
from gateway.config import GatewayConfig, Platform, PlatformConfig
|
||||
from gateway.platforms.base import MessageEvent
|
||||
from gateway.session import SessionEntry, SessionSource
|
||||
|
||||
|
||||
def _make_runner():
|
||||
from gateway.run import GatewayRunner
|
||||
|
||||
runner = object.__new__(GatewayRunner)
|
||||
runner.config = GatewayConfig(
|
||||
platforms={Platform.TELEGRAM: PlatformConfig(enabled=True, token="***")}
|
||||
)
|
||||
runner.adapters = {}
|
||||
runner._voice_mode = {}
|
||||
runner.hooks = SimpleNamespace(emit=AsyncMock(), loaded_hooks=False)
|
||||
runner.session_store = MagicMock()
|
||||
runner.session_store.get_or_create_session.return_value = SessionEntry(
|
||||
session_key="agent:main:telegram:dm:c1:u1",
|
||||
session_id="sess-1",
|
||||
created_at=datetime.now(),
|
||||
updated_at=datetime.now(),
|
||||
platform=Platform.TELEGRAM,
|
||||
chat_type="dm",
|
||||
)
|
||||
runner.session_store.load_transcript.return_value = []
|
||||
runner.session_store.has_any_sessions.return_value = True
|
||||
runner.session_store.append_to_transcript = MagicMock()
|
||||
runner.session_store.rewrite_transcript = MagicMock()
|
||||
runner._running_agents = {}
|
||||
runner._pending_messages = {}
|
||||
runner._pending_approvals = {}
|
||||
runner._session_db = None
|
||||
runner._reasoning_config = None
|
||||
runner._provider_routing = {}
|
||||
runner._fallback_model = None
|
||||
runner._show_reasoning = False
|
||||
runner._is_user_authorized = lambda _source: True
|
||||
runner._set_session_env = lambda _context: None
|
||||
runner._run_agent = AsyncMock(
|
||||
return_value={
|
||||
"final_response": "planned",
|
||||
"messages": [],
|
||||
"tools": [],
|
||||
"history_offset": 0,
|
||||
"last_prompt_tokens": 0,
|
||||
}
|
||||
)
|
||||
return runner
|
||||
|
||||
|
||||
def _make_event(text="/plan"):
|
||||
return MessageEvent(
|
||||
text=text,
|
||||
source=SessionSource(
|
||||
platform=Platform.TELEGRAM,
|
||||
user_id="u1",
|
||||
chat_id="c1",
|
||||
user_name="tester",
|
||||
chat_type="dm",
|
||||
),
|
||||
message_id="m1",
|
||||
)
|
||||
|
||||
|
||||
def _make_plan_skill(skills_dir):
|
||||
skill_dir = skills_dir / "plan"
|
||||
skill_dir.mkdir(parents=True, exist_ok=True)
|
||||
(skill_dir / "SKILL.md").write_text(
|
||||
"""---
|
||||
name: plan
|
||||
description: Plan mode skill.
|
||||
---
|
||||
|
||||
# Plan
|
||||
|
||||
Use the current conversation context when no explicit instruction is provided.
|
||||
Save plans under the active workspace's .hermes/plans directory.
|
||||
"""
|
||||
)
|
||||
|
||||
|
||||
class TestGatewayPlanCommand:
|
||||
@pytest.mark.asyncio
|
||||
async def test_plan_command_loads_skill_and_runs_agent(self, monkeypatch, tmp_path):
|
||||
import gateway.run as gateway_run
|
||||
|
||||
runner = _make_runner()
|
||||
event = _make_event("/plan Add OAuth login")
|
||||
|
||||
monkeypatch.setattr(gateway_run, "_resolve_runtime_agent_kwargs", lambda: {"api_key": "***"})
|
||||
monkeypatch.setattr(
|
||||
"agent.model_metadata.get_model_context_length",
|
||||
lambda *_args, **_kwargs: 100_000,
|
||||
)
|
||||
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
_make_plan_skill(tmp_path)
|
||||
scan_skill_commands()
|
||||
result = await runner._handle_message(event)
|
||||
|
||||
assert result == "planned"
|
||||
forwarded = runner._run_agent.call_args.kwargs["message"]
|
||||
assert "Plan mode skill" in forwarded
|
||||
assert "Add OAuth login" in forwarded
|
||||
assert ".hermes/plans" in forwarded
|
||||
assert str(tmp_path / "plans") not in forwarded
|
||||
assert "active workspace/backend cwd" in forwarded
|
||||
assert "Runtime note:" in forwarded
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_plan_command_appears_in_help_output_via_skill_listing(self, tmp_path):
|
||||
runner = _make_runner()
|
||||
event = _make_event("/help")
|
||||
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
_make_plan_skill(tmp_path)
|
||||
scan_skill_commands()
|
||||
result = await runner._handle_help_command(event)
|
||||
|
||||
assert "/plan" in result
|
||||
|
|
@ -189,11 +189,14 @@ class TestGatewayHelpLines:
|
|||
assert len(lines) > 10
|
||||
|
||||
def test_excludes_cli_only_commands_without_config_gate(self):
|
||||
import re
|
||||
lines = gateway_help_lines()
|
||||
joined = "\n".join(lines)
|
||||
for cmd in COMMAND_REGISTRY:
|
||||
if cmd.cli_only and not cmd.gateway_config_gate:
|
||||
assert f"`/{cmd.name}" not in joined, \
|
||||
# Word-boundary match so `/reload` doesn't match `/reload-mcp`
|
||||
pattern = rf'`/{re.escape(cmd.name)}(?![-_\w])'
|
||||
assert not re.search(pattern, joined), \
|
||||
f"cli_only command /{cmd.name} should not be in gateway help"
|
||||
|
||||
def test_includes_alias_note_for_bg(self):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue