From 2a4556449e1bdb85ea4a01dc05a37bf604c42574 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Fri, 18 Sep 2026 04:53:06 -0700 Subject: [PATCH] fix(tui_gateway): commands.catalog keeps a discovery-failure warning over the skill collision note A skill whose name collides with a built-in command produced a collision note that overwrote a quick_commands / plugin discovery failure already in `warning`, so a broken config.yaml was silently reported as a mere name collision. Failures now win; the collision note only fills an otherwise empty warning. `_catalog_skills` still runs unconditionally so skills keep listing when a loader failed. --- .../test_skill_collision_visibility.py | 19 +++++++++++++++++++ tui_gateway/methods_tools.py | 7 ++++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/tests/hermes_cli/test_skill_collision_visibility.py b/tests/hermes_cli/test_skill_collision_visibility.py index 0998ef7ed3..24d71b8d99 100644 --- a/tests/hermes_cli/test_skill_collision_visibility.py +++ b/tests/hermes_cli/test_skill_collision_visibility.py @@ -57,3 +57,22 @@ def test_built_in_name_collision_is_visible_on_every_listing_surface(monkeypatch gateway_commands = _exec_commands(CommandContext(args="", options={"page_size": 500})).text assert f"⚠ {NOTE}" in gateway_commands and "`/tidy-notes`" in gateway_commands + + +def test_catalog_discovery_failure_warning_outranks_the_collision_note(monkeypatch): + """A colliding skill must not hide a real discovery failure: the failure stays in ``warning``.""" + import tools.skills_tool as skills_tool + from tui_gateway import server + + _write_skill("handoff") + _write_skill("tidy-notes") # control: skills still list when a loader failed + monkeypatch.setattr(skills_tool, "_SKILLS_CACHE", {}) + + def _broken_cfg(): + raise RuntimeError("config.yaml unreadable") + + monkeypatch.setattr(server, "_load_cfg", _broken_cfg) + + catalog = server._methods["commands.catalog"](1, {})["result"] + assert catalog["warning"] == "quick_commands discovery unavailable: config.yaml unreadable" + assert "/tidy-notes" in catalog["skills"] diff --git a/tui_gateway/methods_tools.py b/tui_gateway/methods_tools.py index b260bca1e3..b2c7405e55 100644 --- a/tui_gateway/methods_tools.py +++ b/tui_gateway/methods_tools.py @@ -442,8 +442,8 @@ def _catalog_skills(cat: _Catalog, skills: dict[str, dict]) -> str: @_rpc("commands.catalog", 5020) def _(rid, params: dict) -> dict: """Registry-backed slash metadata, categorized, no aliases. Discovery failures land in ``warning`` - (skills' message wins, then quick commands', then plugins'); with no failure it carries the - built-in-name collision notice for skills that have no ``/`` (empty when none). Skill + (skills' message wins, then quick commands', then plugins'); only with no failure does it carry + the built-in-name collision notice for skills that have no ``/`` (empty when none). Skill discovery is bound to the calling session's profile and workspace (``_completion_cwd``: its record, else the cwd a new session would be seeded with) so project-local skills register for the repo the session is actually in (#114359).""" @@ -461,7 +461,8 @@ def _(rid, params: dict) -> dict: skills: dict[str, dict] = {} try: with _session_home_scope(_sessions.get(params.get("session_id", "")), cwd=_completion_cwd(params)): - warning = _catalog_skills(cat, skills) or warning + collision_note = _catalog_skills(cat, skills) # always runs: skills must list even when a loader failed + warning = warning or collision_note except Exception as e: warning = f"skill discovery unavailable: {e}" return _ok(rid, {