From 428248677e83ca33ee8b0770b7b7afa8768ababf Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sat, 4 Jul 2026 09:31:42 +0200 Subject: [PATCH] LazyToolBox.create_tool: self-heal an index miss for on-disk files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shed installs never survived lazy mode: metadata generation (installed_repository_metadata_manager.get_repository_tools_tups) loads each freshly cloned tool before add_to_tool_panel persists the conf and populates the index, so create_tool raised on every install and the repository landed broken (data managers, workflow test tools, fastp panel tests). A miss for a file that exists on disk now populates that single path — still through the single-writer populator; populate_store_inline synthesizes a DiscoveredTool for requested paths outside every conf and threads the caller's guid so the ad-hoc entry is keyed like its eventual conf-driven replacement — then reloads the index and retries. Misses for files that don't exist still raise. --- lib/galaxy/tool_source_store/populator.py | 32 ++++++++++++++- lib/galaxy/tools/lazy_toolbox.py | 48 ++++++++++++++++++++--- test/unit/app/tools/test_lazy_tool.py | 34 ++++++++++++++++ 3 files changed, 108 insertions(+), 6 deletions(-) diff --git a/lib/galaxy/tool_source_store/populator.py b/lib/galaxy/tool_source_store/populator.py index 42afb45f3e1..ba4b50bfd65 100644 --- a/lib/galaxy/tool_source_store/populator.py +++ b/lib/galaxy/tool_source_store/populator.py @@ -607,6 +607,7 @@ def populate_store_inline( broadcast: bool = False, target: str | None = None, prune: bool = False, + path_guids: dict[str, str | None] | None = None, ) -> dict[str, int]: """In-process populator entry. @@ -676,6 +677,30 @@ def populate_store_inline( if paths is not None: paths_set = {str(p) for p in paths} tool_specs = [(d, n) for d, n in tool_specs if d.path in paths_set] + # Requested paths the conf walk can't reach — a freshly cloned shed + # repository whose conf entry isn't persisted yet (metadata + # generation loads its tools first), or any other ad-hoc load. + # Synthesize their DiscoveredTool so partial populates index them + # before the conf catches up; the next conf-driven populate + # overwrites these entries with full conf context. + covered = {d.path for d, _ in tool_specs} + if DEFAULT_STORE_NAME in writable_names: + for p in sorted(paths_set - covered): + if not Path(p).exists(): + continue + guid = (path_guids or {}).get(p) + tool_specs.append( + ( + DiscoveredTool( + path=p, + tool_conf="adhoc", + tool_path=None, + guid=guid, + is_shed_tool=guid is not None, + ), + DEFAULT_STORE_NAME, + ) + ) log.info(f"Restricted to {len(tool_specs)} tools matching {len(paths_set)} requested path(s)") if pattern: @@ -839,13 +864,17 @@ def populate_for_paths( paths: list[str], *, rebuild_whoosh: bool = True, + path_guids: dict[str, str | None] | None = None, ) -> dict[str, int]: """Partial-update populator entry for shed installs. Restricts the scan to ``paths`` (typically the freshly-written tool files of a newly-installed repository), adds/replaces their index entries, and broadcasts ``reload_tool_source_cache`` so peer Galaxy - processes pick up the new tools. + processes pick up the new tools. ``path_guids`` supplies the guid for + paths that no persisted conf covers yet (install-time metadata + generation), so the ad-hoc entries are keyed like their eventual + conf-driven replacements. """ return populate_store_inline( config, @@ -853,6 +882,7 @@ def populate_for_paths( paths=paths, rebuild_whoosh=rebuild_whoosh, broadcast=True, + path_guids=path_guids, ) diff --git a/lib/galaxy/tools/lazy_toolbox.py b/lib/galaxy/tools/lazy_toolbox.py index 4cc1e9a536e..17650fa0452 100644 --- a/lib/galaxy/tools/lazy_toolbox.py +++ b/lib/galaxy/tools/lazy_toolbox.py @@ -33,7 +33,10 @@ from galaxy.tool_source_store.index import ( ToolIndex, ToolIndexEntry, ) -from galaxy.tool_source_store.populator import populate_store_inline +from galaxy.tool_source_store.populator import ( + populate_for_paths, + populate_store_inline, +) from galaxy.tool_util.id_util import extract_short_id_from_guid from galaxy.tool_util.ontologies.ontology_data import curated_tool_tags from galaxy.tool_util.parser import get_tool_source @@ -765,12 +768,21 @@ class LazyToolBox(ToolBox): ``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. + too. A miss for a file that exists on disk self-heals through a + single-path populate (:meth:`_populate_adhoc_path`) — shed installs + load cloned tools during metadata generation, before any conf is + persisted. A miss for anything else is a contract failure and + raises. """ entry = self._resolve_index_entry(config_file, guid) + if entry is None and config_file is not None and os.path.exists(str(config_file)): + # The file is real but the index doesn't know it. The main + # legitimate path here is a shed install: metadata generation + # (``installed_repository_metadata_manager.get_repository_tools_tups``) + # loads the freshly cloned tools *before* ``add_to_tool_panel`` + # persists the conf and populates. Populate this one path — + # still through the single-writer populator — and retry. + entry = self._populate_adhoc_path(str(config_file), guid) if entry is None: raise RuntimeError( "LazyToolBox.create_tool: no index entry for " @@ -802,6 +814,32 @@ class LazyToolBox(ToolBox): """Bypass the disk ``ToolCache`` — see :meth:`load_tool_from_cache`.""" return None + def _populate_adhoc_path(self, config_file: str, guid: str | None) -> ToolIndexEntry | None: + """Index a single on-disk tool file that no conf covers yet. + + Runs the partial populator for the path (threading the guid so the + entry is keyed like its eventual conf-driven replacement), reloads + the index, and retries resolution. Returns the entry, or ``None`` + when the populator couldn't index the file either. + """ + path = os.path.abspath(config_file) + log.info("LazyToolBox: index miss for existing file %s — populating ad hoc (guid=%s)", path, guid) + try: + populate_for_paths( + self.app.config, + self.app.model.context, + [path], + path_guids={path: guid}, + ) + except Exception as e: + log.warning("Ad-hoc populate for %s raised: %s", path, e) + return None + if self._store is not None: + self._store.invalidate_index_cache() + self._tool_index = self._store.load_index() or ToolIndex() + self._rebuild_shed_short_id_map() + return self._resolve_index_entry(config_file, guid) + def _resolve_index_entry(self, config_file, guid: str | None) -> ToolIndexEntry | None: """Find a matching index entry for the (config_file, guid) pair, or ``None``.""" if self._tool_index is None: diff --git a/test/unit/app/tools/test_lazy_tool.py b/test/unit/app/tools/test_lazy_tool.py index 39e31750f43..70b21f970b7 100644 --- a/test/unit/app/tools/test_lazy_tool.py +++ b/test/unit/app/tools/test_lazy_tool.py @@ -287,6 +287,7 @@ def _seam_box(): box._tool_index = ToolIndex() box._store = MagicMock() box._store.get_by_source_path.return_value = None + box._shed_short_id_to_guids = {} box.app = MagicMock() box.app.config.is_admin_user = lambda u: False return box @@ -322,6 +323,39 @@ def test_resolve_index_entry_returns_none_when_nothing_matches(): assert box._resolve_index_entry(None, None) is None +def test_create_tool_populates_adhoc_for_existing_file(tmp_path, monkeypatch): + # Shed installs load cloned tools during metadata generation, before + # any conf is persisted — a miss for an on-disk file populates that + # path instead of raising. + import galaxy.tools.lazy_toolbox as mod + + guid = "toolshed.example.com/repos/owner/repo/cloned/1.0" + tool_file = tmp_path / "cloned.xml" + tool_file.write_text("") + + box = _seam_box() + healed = ToolIndex() + healed.add_entry(_entry(id=guid)) + box._store.load_index.return_value = healed + + calls = {} + + def fake_populate(config, session, paths, path_guids=None, **kwargs): + calls["paths"] = paths + calls["path_guids"] = path_guids + + monkeypatch.setattr(mod, "populate_for_paths", fake_populate) + tool = box.create_tool(config_file=str(tool_file), guid=guid) + assert isinstance(tool, LazyTool) + assert tool.id == guid + import os as _os + + expected_path = _os.path.abspath(str(tool_file)) + assert calls["paths"] == [expected_path] + assert calls["path_guids"] == {expected_path: guid} + box._store.invalidate_index_cache.assert_called() + + 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``.