mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-08 03:01:47 +00:00
test(skills): cover additional rescan paths in skill_commands cache (#14536)
The rescan-on-platform-change fix landed in #18739 ships one regression test that exercises the HERMES_PLATFORM env-var path. Three other code paths in get_skill_commands / _resolve_skill_commands_platform have no direct coverage; this commit adds a regression test for each. - Gateway session context (HERMES_SESSION_PLATFORM via ContextVar): the resolver consults get_session_env after HERMES_PLATFORM, and the gateway sets that variable through set_session_vars (a ContextVar), not os.environ. The test uses set_session_vars / clear_session_vars to drive the actual gateway signal, and the disabled-skill stub reads the same value via get_session_env. A regression that swapped get_session_env for plain os.getenv would still pass an env-var-based test but break concurrent gateway sessions, which is the bug the ContextVar plumbing exists to prevent. - Returning to no-platform-scope (CLI / cron / RL rollouts after a gateway session): the cached telegram view must be dropped and the unfiltered scan repopulated when HERMES_PLATFORM is unset again. - Same-platform cache hit: consecutive calls under the same platform scope must NOT rescan. The rescan trigger is change in scope, not "always re-resolve" — a gateway serving many consecutive telegram requests should pay the scan cost once, not per request. The third test wraps scan_skill_commands with a spy after the cache is primed, so the assertion is on call_count == 0 across three subsequent get_skill_commands() calls. All 39 tests in tests/agent/test_skill_commands.py pass under scripts/run_tests.sh.
This commit is contained in:
parent
fce58cbe2e
commit
d4de7d4179
1 changed files with 131 additions and 0 deletions
|
|
@ -177,6 +177,137 @@ class TestScanSkillCommands:
|
|||
assert "/telegram-only" not in telegram_again
|
||||
assert "/discord-only" in telegram_again
|
||||
|
||||
def test_get_skill_commands_rescans_when_session_platform_changes(self, tmp_path):
|
||||
"""``HERMES_SESSION_PLATFORM`` from the gateway session context must
|
||||
also trigger a rescan, not just ``HERMES_PLATFORM`` (#14536).
|
||||
|
||||
Exercises the real ContextVar path: the gateway sets the active
|
||||
adapter via ``set_session_vars(platform=...)`` and the resolver
|
||||
reads it via ``get_session_env``. Setting ``HERMES_SESSION_PLATFORM``
|
||||
in ``os.environ`` would only test ``get_session_env``'s legacy
|
||||
env-var fallback — a regression that swapped ``get_session_env``
|
||||
for plain ``os.getenv`` would still pass while breaking concurrent
|
||||
gateway sessions, which is the bug the ContextVar plumbing exists
|
||||
to prevent in the first place.
|
||||
"""
|
||||
import agent.skill_commands as sc_mod
|
||||
from agent.skill_commands import get_skill_commands
|
||||
from gateway.session_context import (
|
||||
clear_session_vars,
|
||||
get_session_env,
|
||||
set_session_vars,
|
||||
)
|
||||
|
||||
def _disabled_skills():
|
||||
platform = (
|
||||
os.getenv("HERMES_PLATFORM")
|
||||
or get_session_env("HERMES_SESSION_PLATFORM")
|
||||
)
|
||||
if platform == "telegram":
|
||||
return {"telegram-only"}
|
||||
if platform == "discord":
|
||||
return {"discord-only"}
|
||||
return set()
|
||||
|
||||
with (
|
||||
patch("tools.skills_tool.SKILLS_DIR", tmp_path),
|
||||
patch("tools.skills_tool._get_disabled_skill_names", side_effect=_disabled_skills),
|
||||
patch.object(sc_mod, "_skill_commands", {}),
|
||||
patch.object(sc_mod, "_skill_commands_platform", None),
|
||||
):
|
||||
_make_skill(tmp_path, "shared")
|
||||
_make_skill(tmp_path, "telegram-only")
|
||||
_make_skill(tmp_path, "discord-only")
|
||||
|
||||
# First simulated gateway request: telegram handler.
|
||||
tokens = set_session_vars(platform="telegram")
|
||||
try:
|
||||
telegram_commands = dict(get_skill_commands())
|
||||
finally:
|
||||
clear_session_vars(tokens)
|
||||
|
||||
assert "/shared" in telegram_commands
|
||||
assert "/discord-only" in telegram_commands
|
||||
assert "/telegram-only" not in telegram_commands
|
||||
|
||||
# Second simulated gateway request: discord handler. The cache
|
||||
# was just populated for telegram; the rescan trigger must fire
|
||||
# off the ContextVar change, not just an env-var change.
|
||||
tokens = set_session_vars(platform="discord")
|
||||
try:
|
||||
discord_commands = dict(get_skill_commands())
|
||||
finally:
|
||||
clear_session_vars(tokens)
|
||||
|
||||
assert "/shared" in discord_commands
|
||||
assert "/telegram-only" in discord_commands
|
||||
assert "/discord-only" not in discord_commands
|
||||
|
||||
def test_get_skill_commands_rescans_when_leaving_platform_scope(self, tmp_path, monkeypatch):
|
||||
"""Returning to no-platform-scope (CLI / cron / RL) after a gateway
|
||||
session must rescan so the unfiltered view is repopulated (#14536).
|
||||
|
||||
A long-lived process running both gateway sessions and bare CLI
|
||||
invocations would otherwise stay stuck on whichever platform's
|
||||
filter was last applied.
|
||||
"""
|
||||
import agent.skill_commands as sc_mod
|
||||
from agent.skill_commands import get_skill_commands
|
||||
|
||||
def _disabled_skills():
|
||||
if os.getenv("HERMES_PLATFORM") == "telegram":
|
||||
return {"telegram-only"}
|
||||
return set()
|
||||
|
||||
with (
|
||||
patch("tools.skills_tool.SKILLS_DIR", tmp_path),
|
||||
patch("tools.skills_tool._get_disabled_skill_names", side_effect=_disabled_skills),
|
||||
patch.object(sc_mod, "_skill_commands", {}),
|
||||
patch.object(sc_mod, "_skill_commands_platform", None),
|
||||
):
|
||||
_make_skill(tmp_path, "shared")
|
||||
_make_skill(tmp_path, "telegram-only")
|
||||
|
||||
monkeypatch.setenv("HERMES_PLATFORM", "telegram")
|
||||
telegram_commands = dict(get_skill_commands())
|
||||
assert "/telegram-only" not in telegram_commands
|
||||
|
||||
# Drop back to no platform scope — bare CLI / cron / RL rollouts.
|
||||
monkeypatch.delenv("HERMES_PLATFORM", raising=False)
|
||||
bare_commands = dict(get_skill_commands())
|
||||
|
||||
assert "/telegram-only" in bare_commands
|
||||
assert sc_mod._skill_commands_platform is None
|
||||
|
||||
def test_get_skill_commands_does_not_rescan_when_platform_unchanged(self, tmp_path):
|
||||
"""Same-platform back-to-back calls must hit the cache, not rescan.
|
||||
|
||||
The rescan trigger is *change* in platform scope, not "always
|
||||
re-resolve." A gateway serving consecutive telegram requests must
|
||||
not pay the scan cost for each one.
|
||||
"""
|
||||
import agent.skill_commands as sc_mod
|
||||
from agent.skill_commands import get_skill_commands
|
||||
|
||||
with (
|
||||
patch("tools.skills_tool.SKILLS_DIR", tmp_path),
|
||||
patch.object(sc_mod, "_skill_commands", {}),
|
||||
patch.object(sc_mod, "_skill_commands_platform", None),
|
||||
patch.dict(os.environ, {"HERMES_PLATFORM": "telegram"}),
|
||||
):
|
||||
_make_skill(tmp_path, "shared")
|
||||
# Prime the cache.
|
||||
get_skill_commands()
|
||||
# Spy on rescans during the subsequent same-platform calls.
|
||||
with patch(
|
||||
"agent.skill_commands.scan_skill_commands",
|
||||
wraps=sc_mod.scan_skill_commands,
|
||||
) as scan_spy:
|
||||
get_skill_commands()
|
||||
get_skill_commands()
|
||||
get_skill_commands()
|
||||
assert scan_spy.call_count == 0
|
||||
|
||||
|
||||
def test_special_chars_stripped_from_cmd_key(self, tmp_path):
|
||||
"""Skill names with +, /, or other special chars produce clean cmd keys."""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue