Merge #4051 into stage-mj
This commit is contained in:
@@ -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.
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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(
|
||||
"""
|
||||
|
||||
@@ -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}
|
||||
|
||||
Reference in New Issue
Block a user