diff --git a/nextcloud_mcp_server/client/__init__.py b/nextcloud_mcp_server/client/__init__.py index 178327ea..db92b853 100644 --- a/nextcloud_mcp_server/client/__init__.py +++ b/nextcloud_mcp_server/client/__init__.py @@ -199,6 +199,33 @@ class NextcloudClient: return response.json() + async def get_enabled_apps(self) -> set[str]: + """Return the set of app ids enabled for the authenticated user. + + Uses the per-user core navigation endpoint, which lists only apps the + current user can access (respecting group restrictions). The vector + scanner uses this to skip polling apps the user lacks, which would 404 + and flood tenant logs. Preferred over ``/cloud/capabilities`` because + the News app advertises no capability and so never appears there. + """ + response = await self._client.get( + "/ocs/v2.php/core/navigation/apps", + headers={"OCS-APIRequest": "true", "Accept": "application/json"}, + ) + response.raise_for_status() + data = response.json() + entries = data.get("ocs", {}).get("data", []) or [] + enabled: set[str] = set() + for entry in entries: + # ``app`` is the canonical app id; ``id`` matches it for the apps we + # gate. Union both so an unexpected nav-entry shape never hides an + # enabled app — a false "disabled" would skip real indexing. + for key in ("app", "id"): + value = entry.get(key) + if value: + enabled.add(value) + return enabled + async def notes_search_notes(self, *, query: str): """Search notes using token-based matching with relevance ranking.""" all_notes = self.notes.get_all_notes() diff --git a/nextcloud_mcp_server/vector/scanner.py b/nextcloud_mcp_server/vector/scanner.py index 2fbe2109..8fc7c707 100644 --- a/nextcloud_mcp_server/vector/scanner.py +++ b/nextcloud_mcp_server/vector/scanner.py @@ -247,6 +247,28 @@ async def scanner_task( logger.info("Scanner task stopped - stream closed") +async def _get_enabled_apps_or_none( + nc_client: NextcloudClient, user_id: str, scan_id: int +) -> set[str] | None: + """Enabled-app id set for gating, or ``None`` when detection fails. + + ``None`` signals "couldn't determine" — callers must then scan every app + (the prior behaviour), so a transient navigation-endpoint failure never + silently halts indexing. The per-app 404 guards in ``scan_user_documents`` + remain the safety net for that fallback path. + """ + try: + return await nc_client.get_enabled_apps() + except Exception as e: + logger.warning( + "[SCAN-%s] Could not determine enabled apps for %s (%s); scanning all apps", + scan_id, + user_id, + e, + ) + return None + + async def scan_user_documents( user_id: str, send_stream: TaskProducer, @@ -328,6 +350,14 @@ async def scan_user_documents( logger.debug("Found %s indexed documents in Qdrant", len(indexed_doc_ids)) + # Determine which apps are enabled for this user so we skip polling + # apps they lack — those polls 404 and flood tenant logs. ``None`` means + # detection failed: fall back to scanning every app (prior behaviour). + enabled_apps = await _get_enabled_apps_or_none(nc_client, user_id, scan_id) + + def _app_enabled(app_id: str) -> bool: + return enabled_apps is None or app_id in enabled_apps + # Notes (isolated so an uninstalled or disabled Notes app — whose API # returns 404 — cannot abort scanning of the other apps; this mirrors the # per-app try/except guards already wrapping files/news/deck below). @@ -336,29 +366,36 @@ async def scan_user_documents( current_time = time.time() queued = 0 - try: - queued += await scan_notes( - user_id=user_id, - send_stream=send_stream, - nc_client=nc_client, - initial_sync=initial_sync, - scan_id=scan_id, - prune_before=prune_before, - indexed_doc_ids=indexed_doc_ids, - grace_period=grace_period, - current_time=current_time, - ) - except HTTPStatusError as e: - if e.response.status_code == 404: - logger.info( - "[SCAN-%s] Notes app unavailable for %s (HTTP 404); skipping notes", - scan_id, - user_id, + if _app_enabled("notes"): + try: + queued += await scan_notes( + user_id=user_id, + send_stream=send_stream, + nc_client=nc_client, + initial_sync=initial_sync, + scan_id=scan_id, + prune_before=prune_before, + indexed_doc_ids=indexed_doc_ids, + grace_period=grace_period, + current_time=current_time, ) - else: + except HTTPStatusError as e: + if e.response.status_code == 404: + logger.info( + "[SCAN-%s] Notes app unavailable for %s (HTTP 404); skipping notes", + scan_id, + user_id, + ) + else: + logger.warning("Failed to scan notes for %s: %s", user_id, e) + except Exception as e: logger.warning("Failed to scan notes for %s: %s", user_id, e) - except Exception as e: - logger.warning("Failed to scan notes for %s: %s", user_id, e) + else: + logger.debug( + "[SCAN-%s] Notes app not enabled for %s; skipping notes", + scan_id, + user_id, + ) if initial_sync: logger.info("Sent %s documents for initial sync: %s", queued, user_id) @@ -666,31 +703,45 @@ async def scan_user_documents( # Scan News items (starred + unread) news_queued = 0 - try: - news_queued = await scan_news_items( - user_id=user_id, - send_stream=send_stream, - nc_client=nc_client, - initial_sync=initial_sync, - scan_id=scan_id, + if _app_enabled("news"): + try: + news_queued = await scan_news_items( + user_id=user_id, + send_stream=send_stream, + nc_client=nc_client, + initial_sync=initial_sync, + scan_id=scan_id, + ) + queued += news_queued + except Exception as e: + logger.warning("Failed to scan news items for %s: %s", user_id, e) + else: + logger.debug( + "[SCAN-%s] News app not enabled for %s; skipping news items", + scan_id, + user_id, ) - queued += news_queued - except Exception as e: - logger.warning("Failed to scan news items for %s: %s", user_id, e) # Scan Deck cards deck_queued = 0 - try: - deck_queued = await scan_deck_cards( - user_id=user_id, - send_stream=send_stream, - nc_client=nc_client, - initial_sync=initial_sync, - scan_id=scan_id, + if _app_enabled("deck"): + try: + deck_queued = await scan_deck_cards( + user_id=user_id, + send_stream=send_stream, + nc_client=nc_client, + initial_sync=initial_sync, + scan_id=scan_id, + ) + queued += deck_queued + except Exception as e: + logger.warning("Failed to scan deck cards for %s: %s", user_id, e) + else: + logger.debug( + "[SCAN-%s] Deck app not enabled for %s; skipping deck cards", + scan_id, + user_id, ) - queued += deck_queued - except Exception as e: - logger.warning("Failed to scan deck cards for %s: %s", user_id, e) if queued > 0: logger.info( diff --git a/tests/unit/client/test_nextcloud_client.py b/tests/unit/client/test_nextcloud_client.py index 46f6846b..14ffae61 100644 --- a/tests/unit/client/test_nextcloud_client.py +++ b/tests/unit/client/test_nextcloud_client.py @@ -7,7 +7,7 @@ folders) into a flat list of files. """ from typing import Any -from unittest.mock import AsyncMock +from unittest.mock import AsyncMock, MagicMock import pytest @@ -31,6 +31,70 @@ def _make_client() -> Any: pytestmark = pytest.mark.unit +def _navigation_response(entries: list[dict]) -> MagicMock: + """Build a mocked OCS v2 ``core/navigation/apps`` response.""" + response = MagicMock() + response.status_code = 200 + response.raise_for_status = MagicMock() + response.json.return_value = {"ocs": {"meta": {}, "data": entries}} + return response + + +class TestGetEnabledApps: + async def test_returns_app_ids_from_navigation(self): + client = _make_client() + client._client = AsyncMock() + client._client.get = AsyncMock( + return_value=_navigation_response( + [ + {"id": "files", "app": "files"}, + {"id": "notes", "app": "notes"}, + {"id": "deck", "app": "deck"}, + {"id": "news", "app": "news"}, + ] + ) + ) + + apps = await client.get_enabled_apps() + + assert apps == {"files", "notes", "deck", "news"} + # Hits the per-user navigation endpoint, not capabilities. + assert ( + client._client.get.await_args.args[0] == "/ocs/v2.php/core/navigation/apps" + ) + + async def test_unions_id_and_app_keys(self): + """When ``id`` and ``app`` differ, both are collected so an enabled + app is never hidden by an unexpected nav-entry id.""" + client = _make_client() + client._client = AsyncMock() + client._client.get = AsyncMock( + return_value=_navigation_response([{"id": "files_sharing", "app": "files"}]) + ) + + apps = await client.get_enabled_apps() + + assert apps == {"files", "files_sharing"} + + async def test_empty_navigation_returns_empty_set(self): + client = _make_client() + client._client = AsyncMock() + client._client.get = AsyncMock(return_value=_navigation_response([])) + + assert await client.get_enabled_apps() == set() + + async def test_skips_entries_missing_both_keys(self): + client = _make_client() + client._client = AsyncMock() + client._client.get = AsyncMock( + return_value=_navigation_response( + [{"name": "Logout", "href": "/logout"}, {"app": "notes"}] + ) + ) + + assert await client.get_enabled_apps() == {"notes"} + + class TestNormaliseSearchResult: def test_adds_leading_slash_to_path(self): result = _normalise_search_result( diff --git a/tests/unit/vector/test_scanner_app_gating.py b/tests/unit/vector/test_scanner_app_gating.py new file mode 100644 index 00000000..dbaf08c3 --- /dev/null +++ b/tests/unit/vector/test_scanner_app_gating.py @@ -0,0 +1,59 @@ +"""Unit tests for the vector scanner's enabled-app gating helper. + +``scan_user_documents`` skips polling apps the user doesn't have enabled (those +polls 404 and flood tenant logs). ``_get_enabled_apps_or_none`` resolves the +enabled-app set, returning ``None`` on any failure so the caller falls back to +scanning every app (the prior behaviour) rather than silently halting indexing. +""" + +from unittest.mock import AsyncMock + +import pytest +from httpx import HTTPStatusError, Request, Response + +from nextcloud_mcp_server.vector.scanner import _get_enabled_apps_or_none + +pytestmark = pytest.mark.unit + + +async def test_returns_enabled_set_on_success(): + nc_client = AsyncMock() + nc_client.get_enabled_apps = AsyncMock(return_value={"files", "notes"}) + + result = await _get_enabled_apps_or_none(nc_client, "alice", scan_id=1234) + + assert result == {"files", "notes"} + + +async def test_returns_none_when_detection_raises(caplog): + nc_client = AsyncMock() + request = Request("GET", "http://nc.test/ocs/v2.php/core/navigation/apps") + nc_client.get_enabled_apps = AsyncMock( + side_effect=HTTPStatusError( + "boom", request=request, response=Response(503, request=request) + ) + ) + + import logging + + caplog.set_level(logging.WARNING, logger="nextcloud_mcp_server.vector.scanner") + result = await _get_enabled_apps_or_none(nc_client, "alice", scan_id=1234) + + # None signals scan-all fallback; the inline gate treats `None` as + # "every app enabled" so indexing never silently stops. + assert result is None + assert "scanning all apps" in caplog.text + + +def test_none_set_enables_every_app(): + """The gate predicate used in scan_user_documents: a None set means + detection failed, so every app must be scanned (back-compat).""" + + def app_enabled(app_id: str, enabled: set[str] | None) -> bool: + return enabled is None or app_id in enabled + + assert app_enabled("news", None) is True + assert app_enabled("deck", None) is True + # And a concrete set gates precisely. + assert app_enabled("news", {"notes"}) is False + assert app_enabled("notes", {"notes"}) is True