From 780c2e2886723bc2bea2b8f8943e58a8abf41a8d Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 18 May 2026 20:28:21 +0200 Subject: [PATCH] discover hidden lib tools so create_tool stays strict on index miss MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ``galaxy.tools.special_tools`` already enumerates Galaxy-internal tool files loaded outside the conf walk (``load_hidden_lib_tool`` → the four ``imp_exp`` / ``data_fetch`` entries, plus ``set_metadata_tool.xml`` which the datatypes registry loads separately). The lazy refactor was missing index entries for those, so the previous step relaxed ``LazyToolBox.create_tool`` to fall through to the eager parent on miss to keep boot working. Add ``hidden_lib_tool_paths()`` in ``special_tools.py``: returns the absolute paths of every hidden-lib tool, including ``set_metadata_tool.xml``. ``SPECIAL_TOOLS`` (consumed by ``load_lib_tools``) stays unchanged so ``set_metadata_tool`` isn't double-loaded — the new ``_EXTRA_HIDDEN_LIB_TOOLS`` dict only feeds the populator-facing helper. ``galaxy.tool_source_store.discover.discover_tools`` walks those paths after the conf + bundled walks, yielding ``DiscoveredTool`` entries with ``tool_conf=""``. The populator's cold-start scan indexes them like any other tool, so the post-boot ``load_hidden_lib_tool`` calls now hit the index branch. With coverage complete, ``LazyToolBox.create_tool`` reverts to the strict raise the plan called for: a miss is now a contract failure (operator forgot to repopulate, or a new ad-hoc tool load didn't get added to ``hidden_lib_tool_paths``). The unit test asserts the raise again. 15/16 integration tests still pass; the remaining failure is the pre-existing flake ``test_run_specific_version_executes_that_version``. --- lib/galaxy/tool_source_store/discover.py | 22 +++++++++++++++++++ lib/galaxy/tools/lazy_toolbox.py | 22 ++++++++++++------- lib/galaxy/tools/special_tools.py | 16 +++++++++----- test/unit/app/tools/test_lazy_tool.py | 27 +++++++----------------- 4 files changed, 55 insertions(+), 32 deletions(-) diff --git a/lib/galaxy/tool_source_store/discover.py b/lib/galaxy/tool_source_store/discover.py index 0f7e1ac0a46..07c9c7eb2fc 100644 --- a/lib/galaxy/tool_source_store/discover.py +++ b/lib/galaxy/tool_source_store/discover.py @@ -363,6 +363,28 @@ def discover_tools( except Exception: pass + # Galaxy-internal "hidden lib" tools (``set_metadata_tool``, the + # ``imp_exp`` history exporters, ``data_fetch``). They're loaded after + # boot via ``toolbox.load_hidden_lib_tool`` rather than from any + # tool_conf, so the conf walk above misses them. Indexing them here + # lets ``LazyToolBox.create_tool`` resolve them on lookup without an + # ad-hoc fall-through. + try: + from galaxy.tools.special_tools import hidden_lib_tool_paths + + for path in hidden_lib_tool_paths(): + if path in seen_paths or not os.path.exists(path): + continue + seen_paths.add(path) + yield DiscoveredTool( + path=path, + tool_conf="", + tool_path=os.path.dirname(path), + is_shed_tool=False, + ) + except Exception as e: + log.debug("Failed to enumerate hidden-lib tool paths: %s", e) + def discover_tool_files( config: "GalaxyAppConfiguration", diff --git a/lib/galaxy/tools/lazy_toolbox.py b/lib/galaxy/tools/lazy_toolbox.py index 435fc9abaf9..b94e85b1a2a 100644 --- a/lib/galaxy/tools/lazy_toolbox.py +++ b/lib/galaxy/tools/lazy_toolbox.py @@ -665,17 +665,23 @@ class LazyToolBox(ToolBox): The populator (cold-start in :meth:`_init_tools_from_configs`, shed installs via ``tool_panel_manager.add_to_tool_panel``) is the single - writer of the index, so the common path lands in the ``LazyTool`` - branch. On miss we delegate to the eager parent: ``load_hidden_lib_tool`` - ( ``set_metadata_tool.xml`` and friends) and any other ad-hoc load - from outside a tool_conf parses eagerly and registers as a real - ``Tool``. No store write — these tools are never re-loaded from the - index. + writer of the index. Hidden lib tools (``set_metadata_tool.xml``, + ``data_fetch``, history import/export) are indexed via + ``galaxy.tools.special_tools.hidden_lib_tool_paths``, so the + post-boot ``load_hidden_lib_tool`` calls resolve through the seam + too. Any miss is therefore a contract failure — operator added a + tool to a conf without re-running the populator, or a code path + introduced a new ad-hoc tool load without adding it to the + hidden-lib list. """ entry = self._resolve_index_entry(config_file, guid) if entry is None: - return super().create_tool( - config_file, tool_shed_repository=tool_shed_repository, guid=guid, **kwds + raise RuntimeError( + "LazyToolBox.create_tool: no index entry for " + f"(config_file={config_file!r}, guid={guid!r}). The populator " + "owns the index — run scripts/tool_source/populate_store.py " + "or, for a new Galaxy-internal lib tool, add it to " + "galaxy.tools.special_tools.hidden_lib_tool_paths()." ) # LazyTool is duck-typed against Tool — the eager pipeline (audited # in plans/witty-drifting-clock.md) only consults attributes the diff --git a/lib/galaxy/tools/special_tools.py b/lib/galaxy/tools/special_tools.py index 81a7cb11c24..7c55c53c303 100644 --- a/lib/galaxy/tools/special_tools.py +++ b/lib/galaxy/tools/special_tools.py @@ -13,8 +13,10 @@ SPECIAL_TOOLS = { } # ``set_metadata_tool`` is loaded separately by -# ``datatypes_registry.load_external_metadata_tool``; listed here so -# ``hidden_lib_tool_paths`` covers every Galaxy-internal tool. +# ``datatypes_registry.load_external_metadata_tool``. Listed here so the +# populator (via ``hidden_lib_tool_paths``) indexes it alongside the +# config-discovered tools — without it, ``LazyToolBox.create_tool`` would +# raise on the post-boot ``load_hidden_lib_tool`` call. _EXTRA_HIDDEN_LIB_TOOLS = { "set metadata": "../datatypes/set_metadata_tool.xml", } @@ -23,12 +25,16 @@ _EXTRA_HIDDEN_LIB_TOOLS = { def hidden_lib_tool_paths() -> list[str]: """Absolute paths of every Galaxy-internal "hidden lib" tool. - Used by :func:`galaxy.tools.source_store.discover.discover_tools` so the - populator indexes these tools alongside the conf-discovered ones. + Used by :func:`galaxy.tool_source_store.discover.discover_tools` so the + populator indexes these tools alongside the conf-discovered ones. The + eager ``load_hidden_lib_tool`` calls that run after boot then resolve + through ``LazyToolBox.create_tool``'s index lookup — the seam stays + strict (raise on miss). """ base = os.path.dirname(__file__) return [ - os.path.abspath(os.path.join(base, p)) for p in (*SPECIAL_TOOLS.values(), *_EXTRA_HIDDEN_LIB_TOOLS.values()) + os.path.abspath(os.path.join(base, p)) + for p in (*SPECIAL_TOOLS.values(), *_EXTRA_HIDDEN_LIB_TOOLS.values()) ] diff --git a/test/unit/app/tools/test_lazy_tool.py b/test/unit/app/tools/test_lazy_tool.py index 76b39e7840a..82303a132e7 100644 --- a/test/unit/app/tools/test_lazy_tool.py +++ b/test/unit/app/tools/test_lazy_tool.py @@ -265,26 +265,15 @@ def test_resolve_index_entry_returns_none_when_nothing_matches(): assert box._resolve_index_entry(None, None) is None -def test_create_tool_falls_through_to_eager_on_index_miss(monkeypatch): - # The populator owns the index for everything that lives in a tool_conf; - # ad-hoc loads (``load_hidden_lib_tool`` for ``set_metadata_tool.xml`` and - # friends) miss the index and delegate to the eager ``ToolBox.create_tool`` - # to parse + register as a real Tool. The lazy path's only responsibility - # is "return a stub on hit"; misses are not the toolbox's contract. +def test_create_tool_raises_on_index_miss(): + # The populator owns the index — including the Galaxy-internal lib + # tools listed in ``galaxy.tools.special_tools.hidden_lib_tool_paths``. + # A miss in ``create_tool`` is a contract failure (operator forgot + # to repopulate, new ad-hoc tool load not added to the lib list); + # raise loudly with a pointer to the fix. box = _seam_box() - called: list[tuple] = [] - - def _fake_super_create_tool(self, config_file, tool_shed_repository=None, guid=None, **kwds): - called.append((config_file, guid)) - return object() - - # Patch the parent's create_tool on the type so super() routes here. - from galaxy.tools import ToolBox - - monkeypatch.setattr(ToolBox, "create_tool", _fake_super_create_tool, raising=True) - result = box.create_tool(config_file="/tools/unknown.xml", guid=None) - assert called == [("/tools/unknown.xml", None)] - assert result is not None + with pytest.raises(RuntimeError, match="no index entry"): + box.create_tool(config_file="/tools/unknown.xml", guid=None) def test_load_tool_from_cache_returns_none():