From 84dfe8a8fc1a177785e6cb2a9a94599e0ca8eba9 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sun, 7 Jun 2026 20:09:47 +0200 Subject: [PATCH] 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):