From b8abe5542c6d2245bf04b58036cc5b3d32e1a3a0 Mon Sep 17 00:00:00 2001 From: chengyongru Date: Thu, 18 Jun 2026 18:05:53 +0800 Subject: [PATCH] Avoid blocking settings on CLI Apps catalog refresh --- nanobot/apps/cli/service.py | 71 ++++++++++---- nanobot/webui/cli_apps_api.py | 46 ++++++++- tests/cli_apps/test_service.py | 30 ++++++ tests/webui/test_cli_apps_api.py | 95 +++++++++++++++++++ .../src/components/settings/SettingsView.tsx | 33 ++++--- webui/src/lib/types.ts | 1 + 6 files changed, 243 insertions(+), 33 deletions(-) create mode 100644 tests/webui/test_cli_apps_api.py diff --git a/nanobot/apps/cli/service.py b/nanobot/apps/cli/service.py index ba9bca43..b4d0c20e 100644 --- a/nanobot/apps/cli/service.py +++ b/nanobot/apps/cli/service.py @@ -407,6 +407,19 @@ class CliAppManager: def _cache_path(self, source: str) -> Path: return self.data_dir / f"{source}_registry_cache.json" + def _cached_registry(self, cache_path: Path) -> tuple[dict[str, Any] | None, float]: + cached = _read_json(cache_path) + if not cached: + return None, 0.0 + data = cached.get("data") + if not isinstance(data, dict): + return None, 0.0 + try: + cached_at = float(cached.get("_cached_at", 0)) + except (TypeError, ValueError): + cached_at = 0.0 + return data, cached_at + def _load_installed(self) -> dict[str, Any]: data = _read_json(self.installed_path) or {} apps = data.get("apps") if isinstance(data.get("apps"), dict) else data @@ -426,39 +439,48 @@ class CliAppManager: *, force_refresh: bool = False, ) -> dict[str, Any]: - cached = _read_json(cache_path) + data, cached_at = self._cached_registry(cache_path) if ( not force_refresh - and cached - and _now() - float(cached.get("_cached_at", 0)) < self.runtime.catalog_ttl_seconds + and data is not None + and _now() - cached_at < self.runtime.catalog_ttl_seconds ): - data = cached.get("data") - if isinstance(data, dict): - return data + return data try: response = httpx.get(url, timeout=15.0, follow_redirects=True) response.raise_for_status() - data = response.json() - if not isinstance(data, dict): + fetched = response.json() + if not isinstance(fetched, dict): raise ValueError("registry response must be an object") except Exception: - if cached and isinstance(cached.get("data"), dict): - return cached["data"] + if data is not None: + return data raise - _write_json(cache_path, {"_cached_at": _now(), "data": data}) - return data + _write_json(cache_path, {"_cached_at": _now(), "data": fetched}) + return fetched - def catalog(self, *, force_refresh: bool = False) -> tuple[list[dict[str, Any]], str | None]: + def catalog( + self, + *, + force_refresh: bool = False, + cache_only: bool = False, + ) -> tuple[list[dict[str, Any]], str | None]: registries: list[tuple[str, str, dict[str, Any]]] = [] for source, url, raw_base, required in _CATALOG_SOURCES: try: - registry = self._fetch_registry( - url, - self._cache_path(source), - force_refresh=force_refresh, - ) + cache_path = self._cache_path(source) + if cache_only: + registry, _ = self._cached_registry(cache_path) + if registry is None: + continue + else: + registry = self._fetch_registry( + url, + cache_path, + force_refresh=force_refresh, + ) except Exception: if required: raise @@ -488,6 +510,15 @@ class CliAppManager: apps_by_name[key] = entry return list(apps_by_name.values()), max(updated_values) if updated_values else None + def catalog_cache_fresh(self) -> bool: + for source, _url, _raw_base, required in _CATALOG_SOURCES: + if not required: + continue + data, cached_at = self._cached_registry(self._cache_path(source)) + if data is None or _now() - cached_at >= self.runtime.catalog_ttl_seconds: + return False + return True + def _manifest_source(self, app: dict[str, Any]) -> str: source = str(app.get("_source") or "harness") if source == "extensions": @@ -674,8 +705,8 @@ class CliAppManager: }, ) - def payload(self, *, force_refresh: bool = False) -> dict[str, Any]: - apps, updated = self.catalog(force_refresh=force_refresh) + def payload(self, *, force_refresh: bool = False, cache_only: bool = False) -> dict[str, Any]: + apps, updated = self.catalog(force_refresh=force_refresh, cache_only=cache_only) installed = self._load_installed() rows = [self._app_payload(app, installed) for app in apps] rows.sort(key=lambda item: (str(item["category"]), str(item["display_name"]).lower())) diff --git a/nanobot/webui/cli_apps_api.py b/nanobot/webui/cli_apps_api.py index b9170d2e..306453d7 100644 --- a/nanobot/webui/cli_apps_api.py +++ b/nanobot/webui/cli_apps_api.py @@ -3,6 +3,8 @@ from __future__ import annotations import re +import threading +import time from typing import Any from nanobot.apps.cli import CliAppError, CliAppManager, CliAppsRuntimeConfig @@ -19,6 +21,39 @@ _CLI_APP_ATTACHMENT_KEYS = ( "logo_url", "brand_color", ) +_CATALOG_REFRESH_RETRY_SECONDS = 60.0 +_catalog_refresh_lock = threading.Lock() +_catalog_refresh_running = False +_catalog_refresh_last_started = 0.0 + + +def _start_catalog_refresh() -> bool: + global _catalog_refresh_last_started, _catalog_refresh_running + now = time.monotonic() + with _catalog_refresh_lock: + if _catalog_refresh_running: + return True + if now - _catalog_refresh_last_started < _CATALOG_REFRESH_RETRY_SECONDS: + return True + _catalog_refresh_running = True + _catalog_refresh_last_started = now + + def refresh() -> None: + global _catalog_refresh_running + try: + _manager().catalog(force_refresh=True) + except Exception: + pass + finally: + with _catalog_refresh_lock: + _catalog_refresh_running = False + + threading.Thread( + target=refresh, + name="nanobot-cli-app-catalog-refresh", + daemon=True, + ).start() + return True def _clip_ws_string(value: Any, limit: int = 240) -> str | None: @@ -77,7 +112,16 @@ def cli_apps_payload(*, installed_only: bool = False) -> dict[str, Any]: manager = _manager() if installed_only: return manager.installed_payload() - return manager.payload() + payload = manager.payload(cache_only=True) + refresh_pending = not manager.catalog_cache_fresh() + if refresh_pending: + _start_catalog_refresh() + if not payload["apps"]: + installed = manager.installed_payload() + if installed["apps"]: + payload = installed + payload["catalog_refresh_pending"] = refresh_pending + return payload def cli_apps_action(action: str, query: QueryParams) -> dict[str, Any]: diff --git a/tests/cli_apps/test_service.py b/tests/cli_apps/test_service.py index 269f5cfd..dd7d3abc 100644 --- a/tests/cli_apps/test_service.py +++ b/tests/cli_apps/test_service.py @@ -270,6 +270,36 @@ def test_optional_extension_registry_failure_does_not_break_payload( assert [app["name"] for app in payload["apps"]] == ["gimp"] +def test_payload_cache_only_does_not_fetch_catalog(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + manager = _manager(tmp_path) + _seed_catalog(manager) + + def fail_get(*args, **kwargs): + raise AssertionError("network should not be used") + + monkeypatch.setattr("nanobot.apps.cli.service.httpx.get", fail_get) + + payload = manager.payload(cache_only=True) + + assert payload["catalog_updated_at"] == "2026-04-18" + assert {app["name"] for app in payload["apps"]} >= {"gimp", "feishu"} + assert manager.catalog_cache_fresh() is True + + +def test_payload_cache_only_without_cache_returns_empty(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + manager = _manager(tmp_path) + + def fail_get(*args, **kwargs): + raise AssertionError("network should not be used") + + monkeypatch.setattr("nanobot.apps.cli.service.httpx.get", fail_get) + + payload = manager.payload(cache_only=True) + + assert payload == {"apps": [], "installed_count": 0, "catalog_updated_at": None} + assert manager.catalog_cache_fresh() is False + + def test_install_dispatches_safe_pip_and_installs_skill( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, diff --git a/tests/webui/test_cli_apps_api.py b/tests/webui/test_cli_apps_api.py new file mode 100644 index 00000000..e3b58e16 --- /dev/null +++ b/tests/webui/test_cli_apps_api.py @@ -0,0 +1,95 @@ +from __future__ import annotations + +from typing import Any + +from nanobot.webui import cli_apps_api + + +class _FakeManager: + def __init__(self, *, fresh: bool, apps: list[dict[str, Any]] | None = None) -> None: + self.fresh = fresh + self.apps = apps or [] + self.payload_calls: list[bool] = [] + + def payload(self, *, cache_only: bool = False) -> dict[str, Any]: + self.payload_calls.append(cache_only) + return { + "apps": list(self.apps), + "installed_count": 0, + "catalog_updated_at": "2026-04-18" if self.apps else None, + } + + def catalog_cache_fresh(self) -> bool: + return self.fresh + + def installed_payload(self) -> dict[str, Any]: + return { + "apps": [ + { + "name": "gimp", + "display_name": "GIMP", + "category": "image", + "description": "Image editing", + "requires": "Python", + "source": "local", + "entry_point": "cli-anything-gimp", + "install_supported": True, + "installed": True, + "available": True, + "status": "installed", + "logo_url": None, + "brand_color": None, + "skill_installed": True, + } + ], + "installed_count": 1, + "catalog_updated_at": None, + } + + +def test_cli_apps_payload_uses_cache_and_marks_refresh_pending(monkeypatch) -> None: + manager = _FakeManager(fresh=False) + refreshes = [] + monkeypatch.setattr(cli_apps_api, "_manager", lambda: manager) + monkeypatch.setattr(cli_apps_api, "_start_catalog_refresh", lambda: refreshes.append(True)) + + payload = cli_apps_api.cli_apps_payload() + + assert manager.payload_calls == [True] + assert refreshes == [True] + assert payload["catalog_refresh_pending"] is True + assert payload["apps"][0]["name"] == "gimp" + + +def test_cli_apps_payload_skips_refresh_when_cache_is_fresh(monkeypatch) -> None: + manager = _FakeManager( + fresh=True, + apps=[ + { + "name": "gimp", + "display_name": "GIMP", + "category": "image", + "description": "Image editing", + "requires": "Python", + "source": "harness", + "entry_point": "cli-anything-gimp", + "install_supported": True, + "installed": False, + "available": False, + "status": "not_installed", + "logo_url": None, + "brand_color": None, + "skill_installed": False, + } + ], + ) + refreshes = [] + monkeypatch.setattr(cli_apps_api, "_manager", lambda: manager) + monkeypatch.setattr(cli_apps_api, "_start_catalog_refresh", lambda: refreshes.append(True)) + + payload = cli_apps_api.cli_apps_payload() + + assert manager.payload_calls == [True] + assert refreshes == [] + assert payload["catalog_refresh_pending"] is False + assert payload["apps"][0]["source"] == "harness" diff --git a/webui/src/components/settings/SettingsView.tsx b/webui/src/components/settings/SettingsView.tsx index ec33c564..d7d411c6 100644 --- a/webui/src/components/settings/SettingsView.tsx +++ b/webui/src/components/settings/SettingsView.tsx @@ -695,22 +695,31 @@ export function SettingsView({ useEffect(() => { if (activeSection !== "apps") return; let cancelled = false; - setCliAppsLoading(true); - fetchCliApps(token) - .then((payload) => { - if (!cancelled) { + let retry: number | null = null; + const loadCliApps = (showLoading: boolean) => { + if (showLoading) setCliAppsLoading(true); + fetchCliApps(token) + .then((payload) => { + if (cancelled) return; + if (payload.catalog_refresh_pending) { + retry = window.setTimeout(() => loadCliApps(false), 2000); + if (payload.apps.length === 0) return; + } setCliApps(payload); setCliAppsError(null); - } - }) - .catch((err) => { - if (!cancelled) setCliAppsError((err as Error).message); - }) - .finally(() => { - if (!cancelled) setCliAppsLoading(false); - }); + setCliAppsLoading(false); + }) + .catch((err) => { + if (!cancelled) { + setCliAppsError((err as Error).message); + setCliAppsLoading(false); + } + }); + }; + loadCliApps(true); return () => { cancelled = true; + if (retry !== null) window.clearTimeout(retry); }; }, [activeSection, token]); diff --git a/webui/src/lib/types.ts b/webui/src/lib/types.ts index 0939aa70..8dbe311d 100644 --- a/webui/src/lib/types.ts +++ b/webui/src/lib/types.ts @@ -605,6 +605,7 @@ export interface CliAppsPayload { apps: CliAppInfo[]; installed_count: number; catalog_updated_at?: string | null; + catalog_refresh_pending?: boolean; last_action?: { ok: boolean; message: string;