From 31a68e92353202190386506e7fb479310cbed085 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Fri, 25 Sep 2026 19:20:51 -0500 Subject: [PATCH] fix(tools): resolve the live plugin catalog once per plugins.list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #119975 display-name lookup called get_live_catalog_entry() inside the per-plugin loop of _plugin_rows, paying a full catalog resolution (load_catalog_live: fetch or cache read + parse of every in-tree catalog yaml, no memoization) once per installed plugin. plugins.manage list went from O(1) to O(installed plugins) catalog resolutions, and a dead catalog host cost one request timeout per plugin — the exact per-candidate cost resolved_removed_entries() exists to eliminate. Hoist the resolution next to pins/versions: catalog_titles() builds {catalog_name: title} in one resolution and _plugin_server_rows reads from the pre-resolved map, mirroring catalog_pins/catalog_versions. Regression test counts load_catalog_live calls across a 3-plugin listing: 3 before, 1 after. --- hermes_cli/plugins_cmd_catalog.py | 10 ++++ .../test_plugins_manage_install.py | 52 +++++++++++++++++++ tui_gateway/methods_tools.py | 19 ++++--- 3 files changed, 74 insertions(+), 7 deletions(-) diff --git a/hermes_cli/plugins_cmd_catalog.py b/hermes_cli/plugins_cmd_catalog.py index e904ee3191..a6f6e5b30c 100644 --- a/hermes_cli/plugins_cmd_catalog.py +++ b/hermes_cli/plugins_cmd_catalog.py @@ -851,6 +851,16 @@ def catalog_pins() -> Dict[str, str]: return {} +def catalog_titles() -> Dict[str, str]: + """``{catalog_name: title}`` for entries that carry one — the Plugins hub server-sentence display + name. One resolution for a whole listing: callers that annotate every installed plugin must not + pay a live-catalog fetch per candidate (see ``resolved_removed_entries``); empty on failure.""" + try: + return {e.name: e.title for e in load_catalog_live() if e.title} + except Exception: + return {} + + def catalog_versions() -> Dict[str, str]: """``{catalog_name: version_label}`` for entries that carry one; empty on failure (best effort).""" try: diff --git a/tests/tui_gateway/test_plugins_manage_install.py b/tests/tui_gateway/test_plugins_manage_install.py index bbb209a876..51b2b12389 100644 --- a/tests/tui_gateway/test_plugins_manage_install.py +++ b/tests/tui_gateway/test_plugins_manage_install.py @@ -74,3 +74,55 @@ def test_plugins_manage_list_reports_desktop_half(tmp_path): assert by_name["media"]["has_desktop_half"] is True assert by_name["snap"]["has_desktop_half"] is False assert by_name["snap"]["servers"] == [] + +def test_plugins_manage_list_resolves_the_live_catalog_once_per_listing(tmp_path): + """The server-sentence display name looks up the curated catalog title, but that lookup must + cost ONE live-catalog resolution per listing, not one per installed plugin: + ``load_catalog_live()`` re-fetches or re-parses the whole catalog on every call (no + memoization), and a dead catalog host costs a request timeout per call — the exact + per-candidate cost ``resolved_removed_entries()`` exists to eliminate.""" + import json + + import hermes_cli.plugin_catalog as plugin_catalog + import hermes_cli.plugins_cmd as plugins_cmd + import hermes_cli.plugins_cmd_catalog as plugins_cmd_catalog + from hermes_cli.plugin_catalog import PluginCatalogEntry + + rows = [] + for i in range(3): + plugin_dir = tmp_path / f"plug{i}" + plugin_dir.mkdir() + (plugin_dir / "plugin.json").write_text(json.dumps({ + "$schema": "https://agent-plugins.org/schemas/1.0.0/plugin.schema.json", + "name": f"plug{i}", + })) + rows.append((f"plug{i}", "1.0", "Plug", "user", plugin_dir, f"plug{i}")) + + entries = [ + PluginCatalogEntry( + name=f"example-{i}", repo="https://example.com/repo", sha="0" * 40, + description="", maintainer="", title=f"Plug {i}") + for i in range(3) + ] + resolver_calls = [] + + def counting_load(): + resolver_calls.append(1) + return entries + + with patch.object(plugins_cmd, "_discover_all_plugins", return_value=rows), \ + patch.object(plugins_cmd, "_get_enabled_set", return_value=set()), \ + patch.object(plugins_cmd, "_get_disabled_set", return_value=set()), \ + patch.object(plugins_cmd_catalog, "catalog_pins", return_value={}), \ + patch.object(plugins_cmd_catalog, "catalog_versions", return_value={}), \ + patch.object(plugins_cmd_catalog, "catalog_install_record", + side_effect=lambda d: {"catalog_name": f"example-{d.name.removeprefix('plug')}"}), \ + patch.object(plugin_catalog, "load_catalog_live", side_effect=counting_load), \ + patch.object(plugins_cmd_catalog, "load_catalog_live", side_effect=counting_load): + resp = server.handle_request({"id": "1", "method": "plugins.manage", "params": {"action": "list"}}) + + assert "error" not in resp + assert len(resp["result"]["plugins"]) == 3 + # ONE resolution for the whole listing (the pre-hoist code paid one per installed plugin). + assert len(resolver_calls) == 1 + diff --git a/tui_gateway/methods_tools.py b/tui_gateway/methods_tools.py index e8836cb96e..d1b7e1abbf 100644 --- a/tui_gateway/methods_tools.py +++ b/tui_gateway/methods_tools.py @@ -1564,7 +1564,10 @@ def _(rid, params: dict) -> dict: # ─── Plugins ───────────────────────────────────────────────────────────────── -def _plugin_server_rows(plugin_dir: Path | None, key: str, *, portable: bool) -> list[dict]: +def _plugin_server_rows( + plugin_dir: Path | None, key: str, *, portable: bool, + catalog_titles: dict[str, str] | None = None, +) -> list[dict]: if not portable or plugin_dir is None: return [] package = _tools_mod("hermes_cli.agent_plugins").load_agent_plugin(plugin_dir, plugin_dir) @@ -1578,14 +1581,15 @@ def _plugin_server_rows(plugin_dir: Path | None, key: str, *, portable: bool) -> resolve_key = _tools_mod("tools.mcp_tool_scope")._resolve_server_key # The server sentence's app name: the curated catalog title when the package is a catalog # install, else the manifest name, else the server slug the declaration carries — a raw - # slug reads like an error code (#119975). + # slug reads like an error code (#119975). *catalog_titles* is pre-resolved by the caller + # (one live-catalog resolution per listing): a per-plugin ``get_live_catalog_entry`` would + # re-resolve the whole catalog once per installed plugin. display_name = str(package.manifest.get("name") or "") or None sidecar = _tools_mod("hermes_cli.plugins_cmd_catalog").catalog_install_record(plugin_dir) if sidecar: - entry = _tools_mod("hermes_cli.plugin_catalog").get_live_catalog_entry( - str(sidecar.get("catalog_name") or "")) - if entry is not None and entry.title: - display_name = entry.title + title = (catalog_titles or {}).get(str(sidecar.get("catalog_name") or "")) + if title: + display_name = title rows = [] for name in sorted(declared): internal_name = server_name_for(key, name) @@ -1614,6 +1618,7 @@ def _plugin_rows() -> list[dict]: enabled, disabled = pc._get_enabled_set(), pc._get_disabled_set() pins = cat.catalog_pins() # powers the desktop's "Update to " affordance versions = cat.catalog_versions() + titles = cat.catalog_titles() # server-sentence display names: ONE live-catalog resolution ref_pins = pc._read_install_metadata() # ``--ref`` installs: pinned_sha so the desktop can show the pin out = [] active = pc._category_active_names() @@ -1633,7 +1638,7 @@ def _plugin_rows() -> list[dict]: "has_desktop_half": bool(_dir_path and (_dir_path / "desktop" / "plugin.js").is_file()), # Manifest ``config_schema`` + current values: the Plugins hub renders these as a form. "settings_schema": _tools_mod("hermes_cli.plugins_settings").plugin_settings_fields(key, _dir_path), - "servers": _plugin_server_rows(_dir_path, key, portable=portable), + "servers": _plugin_server_rows(_dir_path, key, portable=portable, catalog_titles=titles), **cat.catalog_row_fields(_dir, pins, versions), **({"pinned_sha": sha} if (sha := pc.pinned_revision(name, ref_pins)) else {})}) return out