diff --git a/src/specify_cli/bundler/services/catalog_stack.py b/src/specify_cli/bundler/services/catalog_stack.py index d1e7fddde9..a6c1d23522 100644 --- a/src/specify_cli/bundler/services/catalog_stack.py +++ b/src/specify_cli/bundler/services/catalog_stack.py @@ -88,17 +88,25 @@ def search(self, query: str = "") -> list[ResolvedBundle]: Results are sorted by bundle id for deterministic output. """ needle = query.strip().lower() - seen: dict[str, ResolvedBundle] = {} + # Resolve each id to its highest-precedence entry FIRST, then filter by + # the query. Claiming an id only when it matches would let a lower- + # precedence entry with the same id surface when the highest-precedence + # one doesn't match the query — but that shadowed entry is not what + # `resolve()`/install would use, so search would advertise a bundle + # (name, version, author) the user can never actually get. + resolved: dict[str, ResolvedBundle] = {} for source in self._sources: for bundle_id, entry in self._entries_for(source).items(): - if bundle_id in seen: + if bundle_id in resolved: continue - if needle and not _matches(entry, needle): - continue - seen[bundle_id] = ResolvedBundle( + resolved[bundle_id] = ResolvedBundle( entry=entry.with_provenance(source), source=source ) - return [seen[k] for k in sorted(seen)] + return [ + resolved[k] + for k in sorted(resolved) + if not needle or _matches(resolved[k].entry, needle) + ] def _matches(entry: CatalogEntry, needle: str) -> bool: diff --git a/tests/integration/test_bundler_catalog_stack.py b/tests/integration/test_bundler_catalog_stack.py index 55af49039b..e9ab8d912f 100644 --- a/tests/integration/test_bundler_catalog_stack.py +++ b/tests/integration/test_bundler_catalog_stack.py @@ -71,6 +71,47 @@ def test_search_dedupes_by_precedence_and_filters(): assert [r.entry.id for r in qa_only] == ["beta"] +def test_search_does_not_surface_a_shadowed_lower_precedence_entry(): + """Search must resolve each id at its highest-precedence source, then + filter — never fall through to a shadowed lower-precedence entry the query + happens to match. + + If the query matched only the lower-precedence copy of an id, search used + to return that copy, even though `resolve()`/install always use the + higher-precedence one. That advertised a bundle (name/version/source) the + user could never actually get. + """ + sources = [_source("high", 1, "install-allowed"), _source("low", 2, "install-allowed")] + payloads = { + # Highest-precedence entry for 'shared' does NOT match "widget". + "high": catalog_payload({ + "shared": catalog_entry_dict( + "shared", name="Alpha Tool", role="developer", + description="nothing relevant", version="2.0.0", + ), + }), + # Lower-precedence entry for the same id DOES match "widget". + "low": catalog_payload({ + "shared": catalog_entry_dict( + "shared", name="Searchable Widget", version="1.0.0", + ), + }), + } + stack = _stack(sources, payloads) + + # resolve() uses the high-precedence entry. + assert stack.resolve("shared").source.id == "high" + + # A query that only the shadowed low-precedence entry matches returns + # nothing — search agrees with resolve(). + assert stack.search("widget") == [] + + # And a query the high-precedence entry matches returns it (from 'high'). + alpha = stack.search("alpha tool") + assert [r.entry.id for r in alpha] == ["shared"] + assert alpha[0].source.id == "high" + + def test_unreachable_source_raises_named_error(): def fetcher(src): raise RuntimeError("boom")