diff --git a/agent/skill_commands.py b/agent/skill_commands.py index 6b776e1f68..9851fdb750 100644 --- a/agent/skill_commands.py +++ b/agent/skill_commands.py @@ -236,7 +236,7 @@ def _load_skill_payload(skill_identifier: str, task_id: str | None = None) -> tu return None try: - from tools.skills_tool import SKILLS_DIR, skill_view + from tools.skills_tool import _skills_dir, skill_view from agent.skill_utils import normalize_skill_lookup_name normalized = normalize_skill_lookup_name(raw_identifier) @@ -262,7 +262,7 @@ def _load_skill_payload(skill_identifier: str, task_id: str | None = None) -> tu skill_dir = Path(abs_skill_dir) elif skill_path: try: - skill_dir = SKILLS_DIR / Path(skill_path).parent + skill_dir = _skills_dir() / Path(skill_path).parent except Exception: skill_dir = None @@ -317,7 +317,7 @@ def _build_skill_message( session_id: str | None = None, ) -> str: """Format a loaded skill into a user/system message payload.""" - from tools.skills_tool import SKILLS_DIR + from tools.skills_tool import _skills_dir content = str(loaded_skill.get("content") or "") @@ -386,7 +386,7 @@ def _build_skill_message( if supporting and skill_dir: try: - skill_view_target = str(skill_dir.relative_to(SKILLS_DIR)) + skill_view_target = str(skill_dir.relative_to(_skills_dir())) except ValueError: # Skill is from an external dir — use the skill name instead skill_view_target = skill_dir.name @@ -441,7 +441,7 @@ def scan_skill_commands() -> Dict[str, Dict[str, Any]]: # each naming the same skill as its own incumbent (#74574). commands: Dict[str, Dict[str, Any]] = {} try: - from tools.skills_tool import SKILLS_DIR, _parse_frontmatter, skill_matches_platform, skill_matches_environment, _get_disabled_skill_names + from tools.skills_tool import _skills_dir, _parse_frontmatter, skill_matches_platform, skill_matches_environment, _get_disabled_skill_names from agent.skill_utils import ( get_external_skills_dirs, get_project_skills_dirs, @@ -456,8 +456,12 @@ def scan_skill_commands() -> Dict[str, Dict[str, Any]]: # Project dirs iterate through the quarantine chokepoint. project_dirs = list(get_project_skills_dirs()) dirs_to_scan = list(project_dirs) - if SKILLS_DIR.exists(): - dirs_to_scan.append(SKILLS_DIR) + # Resolve at call time: the import-time SKILLS_DIR is frozen to the + # launch home, so a multiplexed profile scope (set_hermes_home_override) + # would still scan the default profile's skills (#67277). + skills_dir = _skills_dir() + if skills_dir.exists(): + dirs_to_scan.append(skills_dir) dirs_to_scan.extend(get_external_skills_dirs()) for scan_dir in dirs_to_scan: diff --git a/agent/skill_utils.py b/agent/skill_utils.py index 19b8f0cc93..a3ab4133ec 100644 --- a/agent/skill_utils.py +++ b/agent/skill_utils.py @@ -996,12 +996,15 @@ def normalize_skill_lookup_name(identifier: str) -> str: # Look the primary skills root up on tools.skills_tool at CALL time # (not via get_skills_dir()): callers and tests patch # ``tools.skills_tool.SKILLS_DIR`` and skill_view() itself resolves - # against that module attribute, so normalization must agree with the - # exact root skill_view() will enforce. Import deferred to avoid a - # module cycle (tools.skills_tool imports agent.skill_utils). + # against ``_skills_dir()`` — which honors that patch and otherwise + # follows the live profile-scoped HERMES_HOME (the import-time + # SKILLS_DIR is frozen to the launch home, #67277) — so normalization + # must agree with the exact root skill_view() will enforce. Import + # deferred to avoid a module cycle (tools.skills_tool imports + # agent.skill_utils). try: from tools import skills_tool as _skills_tool - primary_root = Path(_skills_tool.SKILLS_DIR) + primary_root = _skills_tool._skills_dir() except Exception: primary_root = get_skills_dir() diff --git a/gateway/platforms/webhook.py b/gateway/platforms/webhook.py index 1ff741f1e2..aa44aeb2d7 100644 --- a/gateway/platforms/webhook.py +++ b/gateway/platforms/webhook.py @@ -42,6 +42,7 @@ import subprocess import sys import time from collections import deque +from contextlib import nullcontext from typing import Any, Deque, Dict, List, Optional try: @@ -633,6 +634,21 @@ class WebhookAdapter(BasePlatformAdapter): effective_profile = request_profile or "default" return configured_profile == effective_profile + @staticmethod + def _profile_scope(profile: Optional[str]): + """Enter the URL-resolved profile's runtime scope, or a no-op. + + Only a resolved ``/p//`` prefix enters a scope (same helper + the runner wraps ``handle_message`` in); bare routes keep serving the + launch profile exactly as before. + """ + if not profile or not isinstance(profile, str): + return nullcontext() + from gateway.run import _profile_runtime_scope + from hermes_cli.profiles import get_profile_dir + + return _profile_runtime_scope(get_profile_dir(profile)) + async def _handle_webhook(self, request: "web.Request") -> "web.Response": """POST /webhooks/{route_name} — receive and process a webhook event.""" # Hot-reload dynamic subscriptions on each request (mtime-gated, cheap) @@ -784,63 +800,71 @@ class WebhookAdapter(BasePlatformAdapter): } ) - if route_config.get("script"): - # run_route_script shells out (subprocess.run, up to its timeout); - # run it in a worker thread so it can't block the gateway event loop. - keep, transformed_payload = await asyncio.to_thread( - self._route_processor.run_route_script, - route_config.get("script"), - payload, + # The route script, prompt render and skill lookup below read the + # profile's home (skills/, config). The runner only enters the routed + # profile's scope later, around handle_message, so without this they + # ran against the launch (default) profile (#67277). Only a resolved + # /p// enters a scope; bare routes are unchanged. + with self._profile_scope(profile): + if route_config.get("script"): + # run_route_script shells out (subprocess.run, up to its + # timeout); run it in a worker thread so it can't block the + # gateway event loop. to_thread copies the contextvars, so + # the profile scope follows it. + keep, transformed_payload = await asyncio.to_thread( + self._route_processor.run_route_script, + route_config.get("script"), + payload, + ) + if not keep: + logger.info( + "[webhook] script ignored event=%s route=%s", + event_type, + route_name, + ) + return web.json_response( + { + "status": "ignored", + "reason": "script", + "route": route_name, + } + ) + payload = transformed_payload or payload + + # Format prompt from template + prompt_template = route_config.get("prompt", "") + prompt = self._render_prompt( + prompt_template, payload, event_type, route_name ) - if not keep: - logger.info( - "[webhook] script ignored event=%s route=%s", - event_type, - route_name, - ) - return web.json_response( - { - "status": "ignored", - "reason": "script", - "route": route_name, - } - ) - payload = transformed_payload or payload - # Format prompt from template - prompt_template = route_config.get("prompt", "") - prompt = self._render_prompt( - prompt_template, payload, event_type, route_name - ) + # Inject skill content if configured. + # We call build_skill_invocation_message() directly rather than + # using /skill-name slash commands — the gateway's command parser + # would intercept those and break the flow. + skills = route_config.get("skills", []) + if skills: + try: + from agent.skill_commands import ( + build_skill_invocation_message, + get_skill_commands, + ) - # Inject skill content if configured. - # We call build_skill_invocation_message() directly rather than - # using /skill-name slash commands — the gateway's command parser - # would intercept those and break the flow. - skills = route_config.get("skills", []) - if skills: - try: - from agent.skill_commands import ( - build_skill_invocation_message, - get_skill_commands, - ) - - skill_cmds = get_skill_commands() - for skill_name in skills: - cmd_key = f"/{skill_name}" - if cmd_key in skill_cmds: - skill_content = build_skill_invocation_message( - cmd_key, user_instruction=prompt - ) - if skill_content: - prompt = skill_content - break # Load the first matching skill - else: - logger.warning( - "[webhook] Skill '%s' not found", skill_name - ) - except Exception as e: - logger.warning("[webhook] Skill loading failed: %s", e) + skill_cmds = get_skill_commands() + for skill_name in skills: + cmd_key = f"/{skill_name}" + if cmd_key in skill_cmds: + skill_content = build_skill_invocation_message( + cmd_key, user_instruction=prompt + ) + if skill_content: + prompt = skill_content + break # Load the first matching skill + else: + logger.warning( + "[webhook] Skill '%s' not found", skill_name + ) + except Exception as e: + logger.warning("[webhook] Skill loading failed: %s", e) # Build a unique delivery ID delivery_id = request.headers.get( diff --git a/tests/agent/test_skill_commands.py b/tests/agent/test_skill_commands.py index 623f5a5c05..02e7bfd149 100644 --- a/tests/agent/test_skill_commands.py +++ b/tests/agent/test_skill_commands.py @@ -255,6 +255,41 @@ class TestScanSkillCommands: assert "/b-only" in profile_b_commands assert "/a-only" not in profile_b_commands + def test_get_skill_commands_scans_profile_skills_dir_not_frozen_import_dir(self, tmp_path): + """Under a profile home override the scan must read /skills/, + not the launch home's import-time ``SKILLS_DIR`` (#67277): a + multiplexed webhook routed to profile B otherwise sees default's skills. + Deliberately does NOT patch ``tools.skills_tool.SKILLS_DIR``. + """ + import agent.skill_commands as sc_mod + from agent.skill_commands import build_skill_invocation_message, get_skill_commands + from hermes_constants import reset_hermes_home_override, set_hermes_home_override + + profile_b = tmp_path / "profiles" / "b" + _make_skill(profile_b / "skills", "b-only", body="Body of b-only.") + (profile_b / "config.yaml").write_text("{}\n") + + with ( + patch.object(sc_mod, "_skill_commands", {}), + patch.object(sc_mod, "_skill_commands_platform", None), + patch.object(sc_mod, "_skill_commands_home", None), + ): + token = set_hermes_home_override(profile_b) + try: + commands = dict(get_skill_commands()) + assert "/b-only" in commands + # Frozen SKILLS_DIR (the launch home) must not leak in. + launch_dir = str(skills_tool_module._SKILLS_DIR_AT_IMPORT) + assert not any( + info["skill_dir"].startswith(launch_dir) for info in commands.values() + ) + # And the absolute skill_dir round-trips through skill_view + # (normalize_skill_lookup_name must use the same live root). + msg = build_skill_invocation_message("/b-only", user_instruction="go") + finally: + reset_hermes_home_override(token) + assert msg is not None and "Body of b-only." in msg + 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). diff --git a/tests/gateway/test_webhook_adapter.py b/tests/gateway/test_webhook_adapter.py index 4f5cdb1390..7ea8e8acde 100644 --- a/tests/gateway/test_webhook_adapter.py +++ b/tests/gateway/test_webhook_adapter.py @@ -1011,6 +1011,63 @@ class TestMultiplexProfileWebhookAuthentication: ) assert default_profile.status == 404 + @pytest.mark.asyncio + async def test_routed_profile_skills_resolve_under_that_profile( + self, tmp_path, monkeypatch + ): + """A /p// route's ``skills:`` must load from that profile's + skills/ dir (#67277). Before the fix the lookup ran with no profile + scope, so it scanned the launch profile and logged "Skill not found". + """ + import agent.skill_commands as sc_mod + + worker = tmp_path / "profiles" / "worker" + skill_dir = worker / "skills" / "worker-only" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + "---\nname: worker-only\ndescription: w\n---\n\nBody of worker-only.\n" + ) + (worker / "config.yaml").write_text("{}\n") + (worker / ".env").write_text("") + monkeypatch.setattr( + "hermes_cli.profiles.get_profile_dir", lambda name: tmp_path / "profiles" / name + ) + route_secret = "worker-route-secret-abc123" + adapter = _make_adapter( + routes={ + "gh": { + "profile": "worker", + "secret": route_secret, + "prompt": "PR: {action}", + "skills": ["worker-only"], + } + }, + host="127.0.0.1", + ) + self._configure_profiles(adapter, tmp_path, monkeypatch) + seen = [] + + async def _capture(event): + seen.append(event) + + adapter.handle_message = _capture + body = b'{"action":"opened"}' + headers = { + "Content-Type": "application/json", + "X-Hub-Signature-256": _github_signature(body, route_secret), + } + with ( + patch.object(sc_mod, "_skill_commands", {}), + patch.object(sc_mod, "_skill_commands_home", None), + ): + async with TestClient(TestServer(self._app(adapter))) as cli: + resp = await cli.post("/p/worker/webhooks/gh", data=body, headers=headers) + assert resp.status == 202 + await asyncio.sleep(0.05) + assert len(seen) == 1 + assert seen[0].source.profile == "worker" + assert "Body of worker-only." in seen[0].text + def test_route_profile_validation_fails_closed(): assert WebhookAdapter._route_allows_profile({}, None) is True