diff --git a/openhands-agent-server/openhands/agent_server/marketplace_snapshot.py b/openhands-agent-server/openhands/agent_server/marketplace_snapshot.py new file mode 100644 index 0000000000..9ab44c0c06 --- /dev/null +++ b/openhands-agent-server/openhands/agent_server/marketplace_snapshot.py @@ -0,0 +1,82 @@ +"""Shared loading and caching for the public extensions marketplace.""" + +import json +from pathlib import Path +from threading import Lock +from time import monotonic + +from pydantic import ValidationError + +from openhands.sdk.logger import get_logger +from openhands.sdk.marketplace import Marketplace +from openhands.sdk.skills.skill import ( + DEFAULT_MARKETPLACE_PATH, + PUBLIC_SKILLS_REF, + PUBLIC_SKILLS_REPO, +) +from openhands.sdk.skills.utils import ( + get_skills_cache_dir, + update_skills_repository, +) +from openhands.sdk.utils.path import to_posix_path + + +logger = get_logger(__name__) + +_MARKETPLACE_TTL_SECONDS = 300 +_marketplace_cache: dict[str, tuple[float, Marketplace]] = {} +_marketplace_cache_lock = Lock() + + +def load_marketplace_snapshot( + marketplace_path: str = DEFAULT_MARKETPLACE_PATH, +) -> Marketplace | None: + """Return a successfully loaded marketplace, reusing it within the TTL.""" + now = monotonic() + cached = _marketplace_cache.get(marketplace_path) + if cached is not None and now - cached[0] < _MARKETPLACE_TTL_SECONDS: + return cached[1] + + with _marketplace_cache_lock: + now = monotonic() + cached = _marketplace_cache.get(marketplace_path) + if cached is not None and now - cached[0] < _MARKETPLACE_TTL_SECONDS: + return cached[1] + + cache_dir = get_skills_cache_dir() + repo_path = update_skills_repository( + PUBLIC_SKILLS_REPO, PUBLIC_SKILLS_REF, cache_dir + ) + if repo_path is None: + logger.warning("Failed to access public extensions repository") + return None + + marketplace = _load_marketplace(repo_path, marketplace_path) + if marketplace is not None: + _marketplace_cache[marketplace_path] = (monotonic(), marketplace) + return marketplace + + +def _load_marketplace(repo_path: Path, marketplace_path: str) -> Marketplace | None: + try: + return Marketplace.load(repo_path) + except (FileNotFoundError, ValueError) as discovery_error: + marketplace_file = repo_path / marketplace_path + if not marketplace_file.exists(): + logger.warning( + f"Failed to load marketplace via manifest discovery " + f"({discovery_error}); fallback file not found: {marketplace_file}" + ) + return None + + try: + with open(marketplace_file, encoding="utf-8") as file: + data = json.load(file) + return Marketplace.model_validate( + {**data, "path": to_posix_path(repo_path)} + ) + except (json.JSONDecodeError, ValidationError, OSError) as fallback_error: + logger.warning( + f"Failed to load marketplace: {discovery_error}, {fallback_error}" + ) + return None diff --git a/openhands-agent-server/openhands/agent_server/plugins_service.py b/openhands-agent-server/openhands/agent_server/plugins_service.py index 157e3406a1..62e01fc015 100644 --- a/openhands-agent-server/openhands/agent_server/plugins_service.py +++ b/openhands-agent-server/openhands/agent_server/plugins_service.py @@ -13,13 +13,12 @@ front-end can drive both *attach* and *install* and show install state. """ -import json import os from pathlib import Path -from time import monotonic -from pydantic import BaseModel, ValidationError +from pydantic import BaseModel +from openhands.agent_server.marketplace_snapshot import load_marketplace_snapshot from openhands.sdk.logger import get_logger from openhands.sdk.marketplace import Marketplace from openhands.sdk.plugin import ( @@ -33,15 +32,7 @@ uninstall_plugin, update_plugin, ) -from openhands.sdk.skills.skill import ( - DEFAULT_MARKETPLACE_PATH, - PUBLIC_SKILLS_REF, - PUBLIC_SKILLS_REPO, -) -from openhands.sdk.skills.utils import ( - get_skills_cache_dir, - update_skills_repository, -) +from openhands.sdk.skills.skill import DEFAULT_MARKETPLACE_PATH from openhands.sdk.utils.path import to_posix_path @@ -227,26 +218,6 @@ class MarketplacePluginInfo(BaseModel): files: list[str] | None = None -# --------------------------------------------------------------------------- -# Marketplace catalog cache -# --------------------------------------------------------------------------- -# Mirrors the skills marketplace cache: each call would otherwise trigger a git -# fetch (network-bound, multiple seconds). A short TTL avoids that on every tab -# open. Only the catalog structure is cached; ``installed`` is always derived -# fresh from the local FS. Unlike the skills cache, an *empty* result (e.g. a -# transient fetch failure) is NOT cached, so one flaky fetch does not blank the -# catalog for the whole TTL. -# -# Cached entries are full MarketplacePluginInfo objects (including contents) -# stored with ``installed=False``; serving stamps the fresh installed flag via -# a shallow ``model_copy``, so the ``skills``/``files`` lists are shared across -# requests — they are never mutated after construction. -# -# Type: (timestamp, catalog entries with installed=False) or None -_plugin_catalog_cache: tuple[float, list[MarketplacePluginInfo]] | None = None -_PLUGIN_CATALOG_TTL_SECONDS = 300 # 5 minutes - - def service_get_plugins_marketplace_catalog( marketplace_path: str = DEFAULT_MARKETPLACE_PATH, installed_dir: Path | None = None, @@ -257,9 +228,8 @@ def service_get_plugins_marketplace_catalog( true plugins (source under ``./plugins/``), and enriches each with its attachable coordinates and installation status. - The catalog structure is cached for ``_PLUGIN_CATALOG_TTL_SECONDS`` to avoid - a git fetch on every call. The ``installed`` field is always resolved fresh - from the local FS. + The shared marketplace snapshot is cached, while the ``installed`` field is + always resolved fresh from the local FS. Args: marketplace_path: Relative path to the marketplace JSON file. @@ -269,20 +239,8 @@ def service_get_plugins_marketplace_catalog( Returns: List of MarketplacePluginInfo with plugin details and install status. """ - global _plugin_catalog_cache - - now = monotonic() - if ( - _plugin_catalog_cache is not None - and now - _plugin_catalog_cache[0] < _PLUGIN_CATALOG_TTL_SECONDS - ): - entries = _plugin_catalog_cache[1] - else: - entries = _fetch_plugin_catalog_entries(marketplace_path) - # Only cache non-empty results so a transient fetch failure does not - # blank the catalog for the whole TTL. - if entries: - _plugin_catalog_cache = (now, entries) + marketplace = load_marketplace_snapshot(marketplace_path) + entries = _plugin_catalog_entries(marketplace) if marketplace is not None else [] # Always-fresh installed check — local FS scan, not a network call. installed_names = { @@ -309,53 +267,9 @@ def _is_true_plugin(raw_source: object) -> bool: return subpath.startswith(_PLUGINS_SUBPATH_PREFIX) -def _fetch_plugin_catalog_entries( - marketplace_path: str, +def _plugin_catalog_entries( + marketplace: Marketplace, ) -> list[MarketplacePluginInfo]: - """Fetch the marketplace and keep only true plugins. - - Slow path: git fetch + read the marketplace JSON. Returns catalog entries - with ``installed=False`` (the caller stamps the fresh value), enriched with - local contents (``path``/``skills``/``files``) when an entry resolves to a - directory inside the local clone. Returns an empty list on error. - """ - cache_dir = get_skills_cache_dir() - repo_path = update_skills_repository( - PUBLIC_SKILLS_REPO, PUBLIC_SKILLS_REF, cache_dir - ) - - if repo_path is None: - logger.warning("Failed to access public extensions repository") - return [] - - # Primary loader: ``Marketplace.load`` discovers the manifest in - # ``.plugin/`` or ``.claude-plugin/`` — the real OpenHands/extensions layout, - # where ``.plugin/marketplace.json`` points at the published catalog. We must - # NOT gate on ``marketplace_path`` (``marketplaces/default.json``) existing - # first: that file is absent in the current extensions repo, so an early - # return there would blank the catalog even though the manifest is present. - # The explicit ``marketplace_path`` file is only a fallback for layouts that - # ship it instead of a ``.plugin/`` manifest. - try: - marketplace = Marketplace.load(repo_path) - except (FileNotFoundError, ValueError) as e: - marketplace_file = repo_path / marketplace_path - if not marketplace_file.exists(): - logger.warning( - f"Failed to load marketplace via manifest discovery ({e}); " - f"fallback file not found: {marketplace_file}" - ) - return [] - try: - with open(marketplace_file, encoding="utf-8") as f: - data = json.load(f) - marketplace = Marketplace.model_validate( - {**data, "path": to_posix_path(repo_path)} - ) - except (json.JSONDecodeError, ValidationError, OSError) as e2: - logger.warning(f"Failed to load marketplace: {e}, {e2}") - return [] - entries: list[MarketplacePluginInfo] = [] for plugin in marketplace.plugins: if not _is_true_plugin(plugin.source): diff --git a/openhands-agent-server/openhands/agent_server/skills_service.py b/openhands-agent-server/openhands/agent_server/skills_service.py index 0d87f37215..a69473e50e 100644 --- a/openhands-agent-server/openhands/agent_server/skills_service.py +++ b/openhands-agent-server/openhands/agent_server/skills_service.py @@ -14,16 +14,15 @@ sandbox < registered marketplace/public < user < org < project """ -import json import shutil import subprocess import tempfile from dataclasses import dataclass from pathlib import Path -from time import monotonic -from pydantic import BaseModel, ValidationError +from pydantic import BaseModel +from openhands.agent_server.marketplace_snapshot import load_marketplace_snapshot from openhands.sdk.logger import get_logger from openhands.sdk.marketplace import Marketplace from openhands.sdk.marketplace.registration import MarketplaceRegistration @@ -54,7 +53,6 @@ update_skills_repository, ) from openhands.sdk.utils import sanitized_env -from openhands.sdk.utils.path import to_posix_path logger = get_logger(__name__) @@ -638,26 +636,7 @@ class MarketplaceSkillInfo(BaseModel): installed: bool -# --------------------------------------------------------------------------- -# Marketplace catalog cache -# --------------------------------------------------------------------------- -# Each call to service_get_marketplace_catalog triggers a git fetch via -# update_skills_repository, which is a network-bound operation that takes -# multiple seconds. A short TTL cache avoids that hit on every tab open. -# -# Only the catalog structure (name, description, source) is cached; the -# `installed` field is always derived fresh from the local FS so that -# install/uninstall actions are reflected immediately. -# -# Thread safety: concurrent cache misses (cold start or TTL expiry) may -# trigger parallel git fetches, but each fetch is idempotent and produces -# the same result (last writer wins). For this low-traffic endpoint the -# thundering-herd risk is acceptable without an explicit lock. -# -# Type: (timestamp, list-of-(name, description, source)) or None _CatalogEntry = tuple[str, str | None, str] -_catalog_cache: tuple[float, list[_CatalogEntry]] | None = None -_CATALOG_TTL_SECONDS = 300 # 5 minutes def service_get_marketplace_catalog( @@ -669,9 +648,8 @@ def service_get_marketplace_catalog( Loads the marketplace JSON from the public extensions repository and enriches each entry with installation status. - The catalog structure (name, description, source) is cached for - _CATALOG_TTL_SECONDS to avoid a git fetch on every call. The - ``installed`` field is always resolved fresh from the local FS. + The shared marketplace snapshot is cached to avoid a git fetch on every + call. The ``installed`` field is always resolved fresh from the local FS. Args: marketplace_path: Relative path to marketplace JSON file. @@ -682,14 +660,8 @@ def service_get_marketplace_catalog( Returns: List of MarketplaceSkillInfo with skill details and installation status. """ - global _catalog_cache - - now = monotonic() - if _catalog_cache is not None and now - _catalog_cache[0] < _CATALOG_TTL_SECONDS: - entries = _catalog_cache[1] - else: - entries = _fetch_catalog_entries(marketplace_path) - _catalog_cache = (now, entries) + marketplace = load_marketplace_snapshot(marketplace_path) + entries = _catalog_entries(marketplace) if marketplace is not None else [] # Always-fresh installed check — local FS scan, not a network call. installed_names = { @@ -703,43 +675,7 @@ def service_get_marketplace_catalog( ] -def _fetch_catalog_entries(marketplace_path: str) -> list[_CatalogEntry]: - """Fetch marketplace catalog entries from the public extensions repository. - - This is the slow path: it does a git fetch + reads the marketplace JSON. - Results are cached by the caller. - - Returns: - List of (name, description, source) tuples, or an empty list on error. - """ - cache_dir = get_skills_cache_dir() - repo_path = update_skills_repository( - PUBLIC_SKILLS_REPO, PUBLIC_SKILLS_REF, cache_dir - ) - - if repo_path is None: - logger.warning("Failed to access public skills repository") - return [] - - marketplace_file = repo_path / marketplace_path - if not marketplace_file.exists(): - logger.warning(f"Marketplace file not found: {marketplace_file}") - return [] - - try: - marketplace = Marketplace.load(repo_path) - except (FileNotFoundError, ValueError) as e: - # Fallback to loading from specific path - try: - with open(marketplace_file, encoding="utf-8") as f: - data = json.load(f) - marketplace = Marketplace.model_validate( - {**data, "path": to_posix_path(repo_path)} - ) - except (json.JSONDecodeError, ValidationError, OSError) as e2: - logger.warning(f"Failed to load marketplace: {e}, {e2}") - return [] - +def _catalog_entries(marketplace: Marketplace) -> list[_CatalogEntry]: # Build catalog from plugins and skills. # Plugins take priority: if a name appears in both plugins and skills, # the plugin version is used (since plugins are added first). diff --git a/tests/agent_server/test_marketplace_snapshot.py b/tests/agent_server/test_marketplace_snapshot.py new file mode 100644 index 0000000000..5e04b827d6 --- /dev/null +++ b/tests/agent_server/test_marketplace_snapshot.py @@ -0,0 +1,154 @@ +import json +from collections.abc import Iterator +from pathlib import Path +from unittest.mock import Mock + +import pytest + +from openhands.agent_server import marketplace_snapshot, plugins_service, skills_service + + +@pytest.fixture(autouse=True) +def _reset_marketplace_cache() -> Iterator[None]: + marketplace_snapshot._marketplace_cache.clear() + yield + marketplace_snapshot._marketplace_cache.clear() + + +def _write_marketplace(path: Path, name: str) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + json.dumps( + { + "name": name, + "owner": {"name": "Test"}, + "plugins": [ + {"name": "plugin", "source": "./plugins/plugin"}, + {"name": "skill-plugin", "source": "./skills/skill-plugin"}, + ], + "skills": [ + {"name": "plugin", "source": "./skills/plugin"}, + {"name": "standalone", "source": "./skills/standalone"}, + ], + } + ) + ) + + +def test_manifest_discovery_precedes_explicit_path( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + repo = tmp_path / "extensions" + _write_marketplace(repo / ".plugin" / "marketplace.json", "manifest") + _write_marketplace(repo / "marketplaces" / "default.json", "fallback") + monkeypatch.setattr( + marketplace_snapshot, "update_skills_repository", lambda *args: repo + ) + + marketplace = marketplace_snapshot.load_marketplace_snapshot() + + assert marketplace is not None + assert marketplace.name == "manifest" + + +def test_explicit_path_is_used_as_fallback( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + repo = tmp_path / "extensions" + _write_marketplace(repo / "marketplaces" / "custom.json", "fallback") + monkeypatch.setattr( + marketplace_snapshot, "update_skills_repository", lambda *args: repo + ) + + marketplace = marketplace_snapshot.load_marketplace_snapshot( + "marketplaces/custom.json" + ) + + assert marketplace is not None + assert marketplace.name == "fallback" + + +def test_only_successful_snapshots_are_cached( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + repo = tmp_path / "extensions" + _write_marketplace(repo / ".plugin" / "marketplace.json", "manifest") + update = Mock(side_effect=[None, repo]) + monkeypatch.setattr(marketplace_snapshot, "update_skills_repository", update) + + assert marketplace_snapshot.load_marketplace_snapshot() is None + loaded = marketplace_snapshot.load_marketplace_snapshot() + cached = marketplace_snapshot.load_marketplace_snapshot() + + assert loaded is cached + assert update.call_count == 2 + + +def test_parse_failure_is_not_cached( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + invalid_repo = tmp_path / "invalid" + invalid_manifest = invalid_repo / ".plugin" / "marketplace.json" + invalid_manifest.parent.mkdir(parents=True) + invalid_manifest.write_text("not json") + valid_repo = tmp_path / "valid" + _write_marketplace(valid_repo / ".plugin" / "marketplace.json", "manifest") + update = Mock(side_effect=[invalid_repo, valid_repo]) + monkeypatch.setattr(marketplace_snapshot, "update_skills_repository", update) + + assert marketplace_snapshot.load_marketplace_snapshot() is None + assert marketplace_snapshot.load_marketplace_snapshot() is not None + assert update.call_count == 2 + + +def test_skills_and_plugins_share_snapshot( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + repo = tmp_path / "extensions" + _write_marketplace(repo / ".plugin" / "marketplace.json", "manifest") + update = Mock(return_value=repo) + monkeypatch.setattr(marketplace_snapshot, "update_skills_repository", update) + monkeypatch.setattr(skills_service, "service_list_installed_skills", lambda **k: []) + monkeypatch.setattr(plugins_service, "list_installed_plugins", lambda **k: []) + + skills = skills_service.service_get_marketplace_catalog() + plugins = plugins_service.service_get_plugins_marketplace_catalog() + + assert [entry.name for entry in skills] == [ + "plugin", + "skill-plugin", + "standalone", + ] + assert [entry.name for entry in plugins] == ["plugin"] + assert Path(skills[0].source).parts[-2:] == ("plugins", "plugin") + update.assert_called_once() + + +def test_installed_state_is_computed_fresh( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + repo = tmp_path / "extensions" + _write_marketplace(repo / ".plugin" / "marketplace.json", "manifest") + monkeypatch.setattr( + marketplace_snapshot, "update_skills_repository", lambda *args: repo + ) + installed_skill = Mock() + installed_skill.name = "skill-plugin" + installed_plugin = Mock() + installed_plugin.name = "plugin" + skill_installs = Mock(side_effect=[[], [installed_skill]]) + plugin_installs = Mock(side_effect=[[], [installed_plugin]]) + monkeypatch.setattr(skills_service, "service_list_installed_skills", skill_installs) + monkeypatch.setattr(plugins_service, "list_installed_plugins", plugin_installs) + + first_skills = skills_service.service_get_marketplace_catalog() + second_skills = skills_service.service_get_marketplace_catalog() + first_plugins = plugins_service.service_get_plugins_marketplace_catalog() + second_plugins = plugins_service.service_get_plugins_marketplace_catalog() + + first_skill = next(item for item in first_skills if item.name == "skill-plugin") + second_skill = next(item for item in second_skills if item.name == "skill-plugin") + assert first_skill.installed is False + assert second_skill.installed is True + assert first_plugins[0].installed is False + assert second_plugins[0].installed is True diff --git a/tests/agent_server/test_plugins_service.py b/tests/agent_server/test_plugins_service.py index e66423a163..aa24fe8e5c 100644 --- a/tests/agent_server/test_plugins_service.py +++ b/tests/agent_server/test_plugins_service.py @@ -11,7 +11,7 @@ from fastapi import FastAPI from fastapi.testclient import TestClient -from openhands.agent_server import plugins_service +from openhands.agent_server import marketplace_snapshot from openhands.agent_server.plugins_router import plugins_router from openhands.agent_server.plugins_service import ( MarketplacePluginInfo, @@ -23,9 +23,9 @@ @pytest.fixture(autouse=True) def _reset_catalog_cache(): """Reset the module-level TTL cache so tests don't leak entries to each other.""" - plugins_service._plugin_catalog_cache = None + marketplace_snapshot._marketplace_cache.clear() yield - plugins_service._plugin_catalog_cache = None + marketplace_snapshot._marketplace_cache.clear() def _write_marketplace(repo_dir: Path, plugins: list[dict]) -> Path: @@ -74,7 +74,7 @@ def test_catalog_returns_only_true_plugins(tmp_path: Path, monkeypatch): ], ) monkeypatch.setattr( - plugins_service, "update_skills_repository", lambda *a, **k: repo + marketplace_snapshot, "update_skills_repository", lambda *a, **k: repo ) installed = tmp_path / "installed" installed.mkdir() @@ -84,7 +84,7 @@ def test_catalog_returns_only_true_plugins(tmp_path: Path, monkeypatch): # Assert: only the ./plugins/ entry, resolved to local PluginSource coords. assert [p.name for p in catalog] == ["local-plugin"] - assert catalog[0].source.endswith("plugins/local-plugin") + assert Path(catalog[0].source).parts[-2:] == ("plugins", "local-plugin") assert catalog[0].ref is None assert catalog[0].repo_path is None @@ -103,7 +103,7 @@ def test_catalog_loads_from_plugin_manifest_layout(tmp_path: Path, monkeypatch): ) assert not (repo / "marketplaces" / "default.json").exists() monkeypatch.setattr( - plugins_service, "update_skills_repository", lambda *a, **k: repo + marketplace_snapshot, "update_skills_repository", lambda *a, **k: repo ) installed = tmp_path / "installed" installed.mkdir() @@ -113,7 +113,7 @@ def test_catalog_loads_from_plugin_manifest_layout(tmp_path: Path, monkeypatch): # Assert: the true plugin is returned (not an empty catalog), skill excluded. assert [p.name for p in catalog] == ["city-weather"] - assert catalog[0].source.endswith("plugins/city-weather") + assert Path(catalog[0].source).parts[-2:] == ("plugins", "city-weather") def test_catalog_resolves_structured_source_and_excludes_structured_skills( @@ -143,7 +143,7 @@ def test_catalog_resolves_structured_source_and_excludes_structured_skills( ], ) monkeypatch.setattr( - plugins_service, "update_skills_repository", lambda *a, **k: repo + marketplace_snapshot, "update_skills_repository", lambda *a, **k: repo ) installed = tmp_path / "installed" installed.mkdir() @@ -168,7 +168,7 @@ def test_catalog_marks_installed_plugins(tmp_path: Path, monkeypatch): ], ) monkeypatch.setattr( - plugins_service, "update_skills_repository", lambda *a, **k: repo + marketplace_snapshot, "update_skills_repository", lambda *a, **k: repo ) store = tmp_path / "installed" store.mkdir() @@ -205,7 +205,7 @@ def test_catalog_enriches_local_plugin_entries_with_contents( ) (plugin_dir / "README.md").write_text("# Local plugin") monkeypatch.setattr( - plugins_service, "update_skills_repository", lambda *a, **k: repo + marketplace_snapshot, "update_skills_repository", lambda *a, **k: repo ) installed = tmp_path / "installed" installed.mkdir() @@ -245,7 +245,7 @@ def test_catalog_leaves_contents_unset_for_remote_sources(tmp_path: Path, monkey ], ) monkeypatch.setattr( - plugins_service, "update_skills_repository", lambda *a, **k: repo + marketplace_snapshot, "update_skills_repository", lambda *a, **k: repo ) installed = tmp_path / "installed" installed.mkdir() diff --git a/tests/agent_server/test_skills_service.py b/tests/agent_server/test_skills_service.py index e5794b65fa..24c278c07b 100644 --- a/tests/agent_server/test_skills_service.py +++ b/tests/agent_server/test_skills_service.py @@ -881,124 +881,3 @@ def test_skill_load_result_empty(self): assert result.skills == [] assert result.sources == {} - - -class TestMarketplaceCatalogCache: - """Tests for TTL caching in service_get_marketplace_catalog.""" - - def setup_method(self): - """Reset the module-level cache before each test.""" - import openhands.agent_server.skills_service as svc - - svc._catalog_cache = None - - def test_cache_miss_calls_fetch(self): - """First call (cold cache) fetches from the repository.""" - entries = [("github", "GitHub skill", "github:org/repo")] - with ( - patch( - "openhands.agent_server.skills_service._fetch_catalog_entries", - return_value=entries, - ) as mock_fetch, - patch( - "openhands.agent_server.skills_service.service_list_installed_skills", - return_value=[], - ), - ): - from openhands.agent_server.skills_service import ( - service_get_marketplace_catalog, - ) - - result = service_get_marketplace_catalog() - - mock_fetch.assert_called_once() - assert len(result) == 1 - assert result[0].name == "github" - assert result[0].installed is False - - def test_cache_hit_skips_fetch(self): - """Second call within TTL reuses cached entries without another fetch.""" - entries = [("github", "GitHub skill", "github:org/repo")] - with ( - patch( - "openhands.agent_server.skills_service._fetch_catalog_entries", - return_value=entries, - ) as mock_fetch, - patch( - "openhands.agent_server.skills_service.service_list_installed_skills", - return_value=[], - ), - ): - from openhands.agent_server.skills_service import ( - service_get_marketplace_catalog, - ) - - service_get_marketplace_catalog() - service_get_marketplace_catalog() - - mock_fetch.assert_called_once() # only one fetch despite two calls - - def test_installed_status_always_fresh(self): - """installed flag is derived fresh on every call, not from the cache.""" - from unittest.mock import MagicMock - - from openhands.agent_server.skills_service import ( - InstalledSkillInfo, - service_get_marketplace_catalog, - ) - - entries = [("github", "GitHub skill", "github:org/repo")] - installed_skill = MagicMock(spec=InstalledSkillInfo) - installed_skill.name = "github" - - with ( - patch( - "openhands.agent_server.skills_service._fetch_catalog_entries", - return_value=entries, - ), - patch( - "openhands.agent_server.skills_service.service_list_installed_skills", - ) as mock_installed, - ): - # First call: skill not installed - mock_installed.return_value = [] - result1 = service_get_marketplace_catalog() - assert result1[0].installed is False - - # Second call (cache hit): skill now installed - mock_installed.return_value = [installed_skill] - result2 = service_get_marketplace_catalog() - assert result2[0].installed is True - - # service_list_installed_skills called twice (once per request) - assert mock_installed.call_count == 2 - - def test_cache_expires_after_ttl(self): - """After TTL expires, the next call fetches from the repository again.""" - import openhands.agent_server.skills_service as svc - - entries = [("github", "GitHub skill", "github:org/repo")] - with ( - patch( - "openhands.agent_server.skills_service._fetch_catalog_entries", - return_value=entries, - ) as mock_fetch, - patch( - "openhands.agent_server.skills_service.service_list_installed_skills", - return_value=[], - ), - ): - from openhands.agent_server.skills_service import ( - service_get_marketplace_catalog, - ) - - service_get_marketplace_catalog() - # Artificially expire the cache - assert svc._catalog_cache is not None - svc._catalog_cache = ( - svc._catalog_cache[0] - svc._CATALOG_TTL_SECONDS - 1, - entries, - ) - service_get_marketplace_catalog() - - assert mock_fetch.call_count == 2 # fetched again after expiry