From 2e609cbea74065cc3c3a14b784f47f0cd5d4f949 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sun, 7 Jun 2026 19:57:00 +0200 Subject: [PATCH 1/4] fix(vector): gate scanner app polls on per-user enabled apps The vector-sync scanner polled every indexed app (Notes, Files, News, Deck) for every provisioned user on each scan cycle. When a user lacks an app, its REST API returns 404; these were caught (indexing continued) but flooded tenant logs with repeated 404s, scaling with users x disabled-apps x scan-frequency and masking real failures. Add NextcloudClient.get_enabled_apps(), which reads the per-user /ocs/v2.php/core/navigation/apps endpoint (respects group restrictions). Chosen over /cloud/capabilities because the News app advertises no capability and never appears there. scan_user_documents now resolves the enabled-app set once per cycle and skips the Notes/News/Deck scans for apps the user lacks. Files stays unconditional (core Tags API, not a 404 source). Detection failures fall back to scanning every app (prior behaviour), so a transient nav-endpoint blip never silently halts indexing; the per-app 404 guards remain as the safety net. Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/client/__init__.py | 27 ++++ nextcloud_mcp_server/vector/scanner.py | 133 +++++++++++++------ tests/unit/client/test_nextcloud_client.py | 66 ++++++++- tests/unit/vector/test_scanner_app_gating.py | 59 ++++++++ 4 files changed, 243 insertions(+), 42 deletions(-) create mode 100644 tests/unit/vector/test_scanner_app_gating.py 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 From b11d1b17a30eaf4f7bd12e3583e53dad9429e588 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sun, 7 Jun 2026 20:04:00 +0200 Subject: [PATCH 2/4] test(vector): address PR #873 round-1 review - Extract `_app_enabled` to a module-level helper so the gate predicate is unit-tested directly instead of via an inline copy that could drift. - Move `import logging` to module scope in test_scanner_app_gating.py. - Harden `get_enabled_apps` OCS-envelope parsing (`X or {}` / `or []`) so a present-but-null `ocs`/`data` coerces to empty instead of raising on `None.get`; add parametrized malformed-envelope tests. - Use https:// in the test request URL (SonarCloud S5332 hotspot). Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/client/__init__.py | 6 +++- nextcloud_mcp_server/vector/scanner.py | 19 +++++++---- tests/unit/client/test_nextcloud_client.py | 14 ++++++++ tests/unit/vector/test_scanner_app_gating.py | 35 +++++++++++--------- 4 files changed, 51 insertions(+), 23 deletions(-) diff --git a/nextcloud_mcp_server/client/__init__.py b/nextcloud_mcp_server/client/__init__.py index db92b853..41f4c974 100644 --- a/nextcloud_mcp_server/client/__init__.py +++ b/nextcloud_mcp_server/client/__init__.py @@ -214,7 +214,11 @@ class NextcloudClient: ) response.raise_for_status() data = response.json() - entries = data.get("ocs", {}).get("data", []) or [] + # ``X or {}``/``or []`` (not ``.get(k, default)``) so a present-but-null + # ``ocs``/``data`` (``{"ocs": null}``) coerces to empty instead of + # raising AttributeError on ``None.get``. + ocs = data.get("ocs") or {} + entries = 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 diff --git a/nextcloud_mcp_server/vector/scanner.py b/nextcloud_mcp_server/vector/scanner.py index 8fc7c707..b1085aa0 100644 --- a/nextcloud_mcp_server/vector/scanner.py +++ b/nextcloud_mcp_server/vector/scanner.py @@ -269,6 +269,16 @@ async def _get_enabled_apps_or_none( return None +def _app_enabled(app_id: str, enabled_apps: set[str] | None) -> bool: + """Whether ``app_id`` should be scanned for the current user. + + ``enabled_apps is None`` means detection failed — every app is treated as + enabled (the scan-all fallback) so a transient navigation-endpoint failure + never silently halts indexing. + """ + return enabled_apps is None or app_id in enabled_apps + + async def scan_user_documents( user_id: str, send_stream: TaskProducer, @@ -355,9 +365,6 @@ async def scan_user_documents( # 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). @@ -366,7 +373,7 @@ async def scan_user_documents( current_time = time.time() queued = 0 - if _app_enabled("notes"): + if _app_enabled("notes", enabled_apps): try: queued += await scan_notes( user_id=user_id, @@ -703,7 +710,7 @@ async def scan_user_documents( # Scan News items (starred + unread) news_queued = 0 - if _app_enabled("news"): + if _app_enabled("news", enabled_apps): try: news_queued = await scan_news_items( user_id=user_id, @@ -724,7 +731,7 @@ async def scan_user_documents( # Scan Deck cards deck_queued = 0 - if _app_enabled("deck"): + if _app_enabled("deck", enabled_apps): try: deck_queued = await scan_deck_cards( user_id=user_id, diff --git a/tests/unit/client/test_nextcloud_client.py b/tests/unit/client/test_nextcloud_client.py index 14ffae61..58f9eaf4 100644 --- a/tests/unit/client/test_nextcloud_client.py +++ b/tests/unit/client/test_nextcloud_client.py @@ -94,6 +94,20 @@ class TestGetEnabledApps: assert await client.get_enabled_apps() == {"notes"} + @pytest.mark.parametrize("body", [{}, {"ocs": None}, {"ocs": {"data": None}}]) + async def test_malformed_envelope_returns_empty_set(self, body): + """A missing/null ``ocs``/``data`` envelope yields an empty set rather + than raising — the scanner then gates every app off, and its own + fallback (``_get_enabled_apps_or_none``) keeps indexing safe.""" + client = _make_client() + response = MagicMock() + response.raise_for_status = MagicMock() + response.json.return_value = body + client._client = AsyncMock() + client._client.get = AsyncMock(return_value=response) + + assert await client.get_enabled_apps() == set() + class TestNormaliseSearchResult: def test_adds_leading_slash_to_path(self): diff --git a/tests/unit/vector/test_scanner_app_gating.py b/tests/unit/vector/test_scanner_app_gating.py index dbaf08c3..ddca0e11 100644 --- a/tests/unit/vector/test_scanner_app_gating.py +++ b/tests/unit/vector/test_scanner_app_gating.py @@ -1,17 +1,22 @@ -"""Unit tests for the vector scanner's enabled-app gating helper. +"""Unit tests for the vector scanner's enabled-app gating helpers. ``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. +scanning every app (the prior behaviour) rather than silently halting indexing; +``_app_enabled`` is the gate predicate applied per app. """ +import logging 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 +from nextcloud_mcp_server.vector.scanner import ( + _app_enabled, + _get_enabled_apps_or_none, +) pytestmark = pytest.mark.unit @@ -27,33 +32,31 @@ async def test_returns_enabled_set_on_success(): async def test_returns_none_when_detection_raises(caplog): nc_client = AsyncMock() - request = Request("GET", "http://nc.test/ocs/v2.php/core/navigation/apps") + request = Request("GET", "https://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 + # None signals scan-all fallback; _app_enabled 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).""" + """A None set means detection failed, so every app must be scanned.""" + assert _app_enabled("news", None) is True + assert _app_enabled("deck", None) is True - 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 +def test_concrete_set_gates_precisely(): + """A resolved set scans only the apps it contains.""" + enabled = {"notes", "files"} + assert _app_enabled("notes", enabled) is True + assert _app_enabled("news", enabled) is False + assert _app_enabled("deck", enabled) is False From 84dfe8a8fc1a177785e6cb2a9a94599e0ca8eba9 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sun, 7 Jun 2026 20:09:47 +0200 Subject: [PATCH 3/4] fix(vector): address PR #873 round-2 review - Correct the misleading `test_malformed_envelope_returns_empty_set` docstring: an empty-set return does NOT trigger the `_get_enabled_apps_or_none` scan-all fallback (which fires only on exceptions); optional apps are gated off for that cycle, Files unaffected. - Check OCS `meta.status` in `get_enabled_apps`: a 200 carrying `status != "ok"` now raises, so a 200-with-failure envelope routes through the scanner's scan-all fallback instead of silently gating every app off. Add a test for the failure-status raise. Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/client/__init__.py | 9 +++++++++ tests/unit/client/test_nextcloud_client.py | 23 ++++++++++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/nextcloud_mcp_server/client/__init__.py b/nextcloud_mcp_server/client/__init__.py index 41f4c974..c25df187 100644 --- a/nextcloud_mcp_server/client/__init__.py +++ b/nextcloud_mcp_server/client/__init__.py @@ -218,6 +218,15 @@ class NextcloudClient: # ``ocs``/``data`` (``{"ocs": null}``) coerces to empty instead of # raising AttributeError on ``None.get``. ocs = data.get("ocs") or {} + # A 200 carrying ``meta.status != "ok"`` is an OCS-level failure (auth / + # permission edge cases) that ``raise_for_status`` can't see. Raise so + # the scanner's ``_get_enabled_apps_or_none`` catches it and falls back + # to scanning every app, rather than silently gating all apps off for a + # cycle on an empty ``data``. Tolerate a missing/empty meta (our own + # mocks, and any envelope that omits it). + status = (ocs.get("meta") or {}).get("status") + if status not in ("ok", None, ""): + raise ValueError(f"OCS navigation returned status={status!r}") entries = ocs.get("data") or [] enabled: set[str] = set() for entry in entries: diff --git a/tests/unit/client/test_nextcloud_client.py b/tests/unit/client/test_nextcloud_client.py index 58f9eaf4..e437a43b 100644 --- a/tests/unit/client/test_nextcloud_client.py +++ b/tests/unit/client/test_nextcloud_client.py @@ -97,8 +97,10 @@ class TestGetEnabledApps: @pytest.mark.parametrize("body", [{}, {"ocs": None}, {"ocs": {"data": None}}]) async def test_malformed_envelope_returns_empty_set(self, body): """A missing/null ``ocs``/``data`` envelope yields an empty set rather - than raising — the scanner then gates every app off, and its own - fallback (``_get_enabled_apps_or_none``) keeps indexing safe.""" + than raising. NOTE: an empty set does NOT trigger the + ``_get_enabled_apps_or_none`` scan-all fallback (that fires only on + exceptions) — all optional apps are gated off for this scan cycle, with + Files unaffected (unconditional) and the next cycle retrying normally.""" client = _make_client() response = MagicMock() response.raise_for_status = MagicMock() @@ -108,6 +110,23 @@ class TestGetEnabledApps: assert await client.get_enabled_apps() == set() + async def test_ocs_failure_status_raises(self): + """A 200 with ``ocs.meta.status == "failure"`` raises so the scanner's + ``_get_enabled_apps_or_none`` falls back to scanning all apps, instead + of silently gating every app off on the empty ``data`` of a failure + envelope.""" + client = _make_client() + response = MagicMock() + response.raise_for_status = MagicMock() + response.json.return_value = { + "ocs": {"meta": {"status": "failure", "statuscode": 997}, "data": None} + } + client._client = AsyncMock() + client._client.get = AsyncMock(return_value=response) + + with pytest.raises(ValueError, match="failure"): + await client.get_enabled_apps() + class TestNormaliseSearchResult: def test_adds_leading_slash_to_path(self): From 2e25c2723e703adf4f4380e519cb4de8d7428314 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sun, 7 Jun 2026 20:14:13 +0200 Subject: [PATCH 4/4] test(vector): address PR #873 round-3 nits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Simplify the OCS status guard to `if status and status != "ok"` — falsy (missing/None/"") is tolerated more naturally than the explicit tuple. - Add `test_value_error_from_ocs_failure_returns_none`, covering the OCS-failure ValueError flowing through `_get_enabled_apps_or_none` to the scan-all fallback. Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/client/__init__.py | 2 +- tests/unit/vector/test_scanner_app_gating.py | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/nextcloud_mcp_server/client/__init__.py b/nextcloud_mcp_server/client/__init__.py index c25df187..6891e47d 100644 --- a/nextcloud_mcp_server/client/__init__.py +++ b/nextcloud_mcp_server/client/__init__.py @@ -225,7 +225,7 @@ class NextcloudClient: # cycle on an empty ``data``. Tolerate a missing/empty meta (our own # mocks, and any envelope that omits it). status = (ocs.get("meta") or {}).get("status") - if status not in ("ok", None, ""): + if status and status != "ok": # falsy (missing/None/"") tolerated raise ValueError(f"OCS navigation returned status={status!r}") entries = ocs.get("data") or [] enabled: set[str] = set() diff --git a/tests/unit/vector/test_scanner_app_gating.py b/tests/unit/vector/test_scanner_app_gating.py index ddca0e11..6d2da6ad 100644 --- a/tests/unit/vector/test_scanner_app_gating.py +++ b/tests/unit/vector/test_scanner_app_gating.py @@ -48,6 +48,19 @@ async def test_returns_none_when_detection_raises(caplog): assert "scanning all apps" in caplog.text +async def test_value_error_from_ocs_failure_returns_none(): + """A ValueError (e.g. OCS meta.status=='failure' from get_enabled_apps) + routes through the scan-all fallback like any other exception.""" + nc_client = AsyncMock() + nc_client.get_enabled_apps = AsyncMock( + side_effect=ValueError("OCS navigation returned status='failure'") + ) + + result = await _get_enabled_apps_or_none(nc_client, "alice", scan_id=1234) + + assert result is None + + def test_none_set_enables_every_app(): """A None set means detection failed, so every app must be scanned.""" assert _app_enabled("news", None) is True