From 1529dd08419d3161b4f99a620b28da5726ce059b Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Fri, 12 Jun 2026 08:25:59 -0400 Subject: [PATCH] fix(commands): route /reload-skills through the WebUI allowlists (#4043) --- api/commands.py | 54 ++++++++++++++- static/messages.js | 2 +- tests/test_cli_only_slash_commands.py | 31 ++++++++- tests/test_commands_endpoint.py | 98 +++++++++++++++++++++++++++ 4 files changed, 182 insertions(+), 3 deletions(-) diff --git a/api/commands.py b/api/commands.py index 52847ab0..0a3ba5fa 100644 --- a/api/commands.py +++ b/api/commands.py @@ -24,10 +24,12 @@ _NEVER_EXPOSE: frozenset[str] = frozenset({ # Narrow agent-side execution allowlist for /api/commands/exec. _AGENT_COMMAND_ALIASES = { 'reload_mcp': 'reload-mcp', + 'reload_skills': 'reload-skills', 'codex_runtime': 'codex-runtime', } -_ALLOWED_AGENT_COMMANDS = frozenset({'reload-mcp', 'codex-runtime'}) +_ALLOWED_AGENT_COMMANDS = frozenset({'reload-mcp', 'reload-skills', 'codex-runtime'}) _RELOAD_MCP_LOCK = threading.Lock() +_RELOAD_SKILLS_LOCK = threading.Lock() _CODEX_RUNTIME_LOCK = threading.Lock() @@ -120,6 +122,8 @@ def execute_agent_command(command: str) -> str: if canonical == 'reload-mcp': return _run_reload_mcp_command() + if canonical == 'reload-skills': + return _run_reload_skills_command() if canonical == 'codex-runtime': return _run_codex_runtime_command(arg_string) @@ -204,6 +208,54 @@ def _run_reload_mcp_command() -> str: return "\n".join(lines) +def _run_reload_skills_command() -> str: + """Re-scan the installed skills directory and summarize the diff.""" + with _RELOAD_SKILLS_LOCK: + try: + from agent.skill_commands import reload_skills + except Exception as exc: + logger.warning("Failed to import skills runtime for /reload-skills", exc_info=True) + raise RuntimeError("Skills runtime unavailable") from exc + + try: + result = reload_skills() or {} + except Exception as exc: + logger.warning("Failed to reload skills", exc_info=True) + raise RuntimeError("Failed to reload skills") from exc + + added = result.get("added", []) + removed = result.get("removed", []) + unchanged = result.get("unchanged", []) + total = int(result.get("total", 0) or 0) + + def _names(items: Any) -> list[str]: + out: list[str] = [] + for item in items or []: + if isinstance(item, dict): + name = str(item.get("name", "")).strip() + else: + name = str(item).strip() + if name: + out.append(name) + return out + + added_names = _names(added) + removed_names = _names(removed) + + lines = [ + "Reloaded skills from disk.", + f"Added: {len(added_names)}", + f"Removed: {len(removed_names)}", + f"Unchanged: {len(list(unchanged or []))}", + f"Total skills: {total}", + ] + if added_names: + lines.append(f"Added skills: {', '.join(sorted(added_names))}") + if removed_names: + lines.append(f"Removed skills: {', '.join(sorted(removed_names))}") + return "\n".join(lines) + + def execute_plugin_command(command: str) -> str: """Execute a plugin-registered slash command and return printable output. diff --git a/static/messages.js b/static/messages.js index c9973fb8..98e4ac47 100644 --- a/static/messages.js +++ b/static/messages.js @@ -792,7 +792,7 @@ const _sessionTitleProvisionalBySid = new Map(); // their canonical command is registered on the backend (for example // /reload-mcp). Keep this intentionally narrow and include underscore variants // observed by users so typing either form still routes through executeAgentCommand. -const _AGENT_COMMANDS_RUN_ON_WEBUI = new Set(['reload-mcp', 'reload_mcp', 'codex-runtime', 'codex_runtime']); +const _AGENT_COMMANDS_RUN_ON_WEBUI = new Set(['reload-mcp', 'reload_mcp', 'reload-skills', 'reload_skills', 'codex-runtime', 'codex_runtime']); function _clearStaleBusyStateBeforeSend({compressionRunning=false}={}){ if(!S||!S.busy||compressionRunning) return false; diff --git a/tests/test_cli_only_slash_commands.py b/tests/test_cli_only_slash_commands.py index 99c558f5..58a2a3bd 100644 --- a/tests/test_cli_only_slash_commands.py +++ b/tests/test_cli_only_slash_commands.py @@ -120,6 +120,13 @@ def _run_commands_js(script_body: str) -> dict: aliases: ['codex_runtime'], cli_only: false, gateway_only: false + }}, + {{ + name: 'reload-skills', + description: 'Re-scan installed skills', + aliases: ['reload_skills'], + cli_only: false, + gateway_only: false }} ] }}; @@ -252,14 +259,36 @@ def test_send_intercepts_reload_mcp_agent_command_before_agent_round_trip(): assert "executeAgentCommand(text,_agentCmd||{name:_agentCmdName})" in intercept -def test_reload_mcp_and_codex_runtime_webui_intercept_aliases_are_defined_in_js_whitelist(): +def test_reload_mcp_reload_skills_and_codex_runtime_webui_intercept_aliases_are_defined_in_js_whitelist(): assert "'reload-mcp'" in MESSAGES_JS assert "'reload_mcp'" in MESSAGES_JS + assert "'reload-skills'" in MESSAGES_JS + assert "'reload_skills'" in MESSAGES_JS assert "'codex-runtime'" in MESSAGES_JS assert "'codex_runtime'" in MESSAGES_JS assert "if(_agentCmd&&_AGENT_COMMANDS_RUN_ON_WEBUI.has(_agentCmdName))" not in MESSAGES_JS +def test_reload_skills_agent_command_metadata_resolves_alias(): + result = _run_commands_js( + """ + const byName = await getAgentCommandMetadata('reload-skills'); + const byAlias = await getAgentCommandMetadata('reload_skills'); + return { + by_name: byName && byName.name, + by_alias: byAlias && byAlias.name, + cli_only: byAlias && byAlias.cli_only === true + }; + """ + ) + + assert result == { + "by_name": "reload-skills", + "by_alias": "reload-skills", + "cli_only": False, + } + + def test_codex_runtime_agent_command_metadata_resolves_alias(): result = _run_commands_js( """ diff --git a/tests/test_commands_endpoint.py b/tests/test_commands_endpoint.py index 9637a6e5..ef9b4b2d 100644 --- a/tests/test_commands_endpoint.py +++ b/tests/test_commands_endpoint.py @@ -60,6 +60,17 @@ def _install_fake_codex_runtime_switch(monkeypatch): return calls +def _install_fake_skill_commands(monkeypatch, reload_skills): + import sys + agent_pkg = sys.modules.get("agent") or ModuleType("agent") + agent_pkg.__path__ = [] + skill_commands = ModuleType("agent.skill_commands") + skill_commands.reload_skills = reload_skills + monkeypatch.setitem(sys.modules, "agent", agent_pkg) + monkeypatch.setitem(sys.modules, "agent.skill_commands", skill_commands) + return skill_commands + + def _get(path): """GET helper -- returns parsed JSON or raises HTTPError.""" with urllib.request.urlopen(TEST_BASE + path, timeout=10) as r: @@ -153,6 +164,24 @@ def test_commands_exec_runs_reload_mcp_alias(): assert isinstance(body['output'], str) +@requires_agent_modules +def test_commands_exec_runs_reload_skills_command(): + """`/reload-skills` executes through the same narrow shared executor path.""" + status, body = _post('/api/commands/exec', {'command': '/reload-skills'}) + assert status == 200 + assert 'output' in body + assert isinstance(body['output'], str) + + +@requires_agent_modules +def test_commands_exec_runs_reload_skills_alias(): + """Telegram-style underscore alias resolves to reload-skills in the executor.""" + status, body = _post('/api/commands/exec', {'command': '/reload_skills'}) + assert status == 200 + assert 'output' in body + assert isinstance(body['output'], str) + + def test_codex_runtime_command_uses_shared_switch_and_persists(monkeypatch, tmp_path): """`/codex-runtime` executes through the same shared switch as CLI/gateway.""" calls = _install_fake_codex_runtime_switch(monkeypatch) @@ -243,6 +272,75 @@ def test_reload_mcp_error_is_generic(monkeypatch): assert calls == ["shutdown"] +def test_reload_skills_command_formats_helper_diff(monkeypatch): + """`/reload-skills` should summarize the shared helper diff in printable text.""" + def reload_skills(): + return { + "added": [{"name": "incident-review", "description": "desc"}], + "removed": [{"name": "legacy-skill", "description": "old"}], + "unchanged": ["skills", "use"], + "total": 3, + "commands": 3, + } + + _install_fake_skill_commands(monkeypatch, reload_skills) + + from api.commands import execute_agent_command + + output = execute_agent_command('/reload-skills') + + assert output == "\n".join([ + "Reloaded skills from disk.", + "Added: 1", + "Removed: 1", + "Unchanged: 2", + "Total skills: 3", + "Added skills: incident-review", + "Removed skills: legacy-skill", + ]) + + +def test_reload_skills_command_accepts_underscore_alias(monkeypatch): + """Telegram/WebUI underscore spelling routes to the canonical skills reload.""" + calls = [] + + def reload_skills(): + calls.append("reload_skills") + return { + "added": [], + "removed": [], + "unchanged": [], + "total": 0, + "commands": 0, + } + + _install_fake_skill_commands(monkeypatch, reload_skills) + + from api.commands import execute_agent_command + + output = execute_agent_command('/reload_skills') + + assert calls == ["reload_skills"] + assert "Added: 0" in output + assert "Removed: 0" in output + + +def test_reload_skills_error_is_generic(monkeypatch): + """`/reload-skills` failures must return a generic message, not internals.""" + def reload_skills(): + raise RuntimeError("secret_path=C:/Users/Rod/.hermes/skills/private") + + _install_fake_skill_commands(monkeypatch, reload_skills) + + from api.commands import execute_agent_command + + with pytest.raises(RuntimeError) as exc: + execute_agent_command('/reload-skills') + + assert str(exc.value) == "Failed to reload skills" + assert 'secret_path=' not in str(exc.value) + + def test_concurrent_reload_mcp_calls_are_serialized(monkeypatch): """Concurrent `/reload-mcp` calls cannot run shutdown/discover interleaved.""" state = {"active": 0, "max_active": 0}