fix(vector-sync): address round-5 review — partial-failure signal, markers
- purge route: include a "failed" key in the 200 body listing requested doc types that were not purged, so Astrolabe knows consent isn't yet enforced for them (scanner backstop still catches up) - tests: add @pytest.mark.unit / module-level pytestmark to the new test modules so they run under `pytest -m unit`; add a partial-failure route test - capabilities: comment why the cache is keyed per-user despite a global value - semantic/scanner: doc/comment clarifications (sorted-order, eviction timing) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
b0751102d7
commit
d0db530ac9
@@ -117,8 +117,20 @@ async def purge_doc_types_route(request: Request) -> JSONResponse:
|
|||||||
return JSONResponse({"purged": {}})
|
return JSONResponse({"purged": {}})
|
||||||
|
|
||||||
purged = await purge_doc_types(doc_types)
|
purged = await purge_doc_types(doc_types)
|
||||||
logger.info("Vector-sync purge by admin %s: %s", user_id, purged)
|
# Surface a partial-failure signal so Astrolabe knows which types were
|
||||||
return JSONResponse({"purged": purged})
|
# NOT purged (consent not yet enforced for them) — the scanner backstop
|
||||||
|
# still catches these, but the caller shouldn't assume full success.
|
||||||
|
failed = [dt for dt in dict.fromkeys(doc_types) if dt not in purged]
|
||||||
|
body: dict = {"purged": purged}
|
||||||
|
if failed:
|
||||||
|
body["failed"] = failed
|
||||||
|
logger.info(
|
||||||
|
"Vector-sync purge by admin %s: purged=%s failed=%s",
|
||||||
|
user_id,
|
||||||
|
purged,
|
||||||
|
failed,
|
||||||
|
)
|
||||||
|
return JSONResponse(body)
|
||||||
|
|
||||||
except ProvisioningRequiredError as e:
|
except ProvisioningRequiredError as e:
|
||||||
logger.info("Provisioning required for user %s: %s", user_id, e)
|
logger.info("Provisioning required for user %s: %s", user_id, e)
|
||||||
|
|||||||
@@ -28,6 +28,12 @@ logger = logging.getLogger(__name__)
|
|||||||
# changes rarely, but search/scan paths consult it frequently, so trade a little
|
# changes rarely, but search/scan paths consult it frequently, so trade a little
|
||||||
# staleness for keeping the OCS round-trip off the hot path. Mirrors the
|
# staleness for keeping the OCS round-trip off the hot path. Mirrors the
|
||||||
# list_accessible_owners cache in search/access_filter.py.
|
# list_accessible_owners cache in search/access_filter.py.
|
||||||
|
#
|
||||||
|
# Keyed by user_id even though enabled_doc_types is an admin-wide value: the OCS
|
||||||
|
# call is authenticated per-user (and ``installed`` resolves per-user on the
|
||||||
|
# Astrolabe side), so we cache per-user for correctness. The redundancy is
|
||||||
|
# bounded by _CACHE_MAXSIZE; on an admin change all entries reconverge within
|
||||||
|
# one TTL window.
|
||||||
_CACHE_TTL_SECONDS = 30.0
|
_CACHE_TTL_SECONDS = 30.0
|
||||||
_CACHE_MAXSIZE = 1024
|
_CACHE_MAXSIZE = 1024
|
||||||
# user_id -> (monotonic_ts, frozenset[doc_type] | None). None = no restriction.
|
# user_id -> (monotonic_ts, frozenset[doc_type] | None). None = no restriction.
|
||||||
|
|||||||
@@ -64,10 +64,12 @@ def _consent_narrowed_doc_types(
|
|||||||
|
|
||||||
Caller has already established ``allowed is not None`` (a concrete allow-set;
|
Caller has already established ``allowed is not None`` (a concrete allow-set;
|
||||||
``None`` means "no restriction" and is handled by skipping this call). When
|
``None`` means "no restriction" and is handled by skipping this call). When
|
||||||
no explicit ``doc_types`` are requested, restrict to the full allow-set;
|
no explicit ``doc_types`` are requested, restrict to the full allow-set
|
||||||
otherwise intersect (preserving the caller's order). An empty result means
|
(returned ``sorted`` for determinism only — order is a filter, not a ranking
|
||||||
nothing the caller asked for is admin-approved — the caller short-circuits
|
hint); otherwise intersect (preserving the caller's order). An empty result
|
||||||
to an empty response rather than falling through to an all-types search.
|
means nothing the caller asked for is admin-approved — the caller
|
||||||
|
short-circuits to an empty response rather than falling through to an
|
||||||
|
all-types search.
|
||||||
"""
|
"""
|
||||||
if doc_types is None:
|
if doc_types is None:
|
||||||
return sorted(allowed)
|
return sorted(allowed)
|
||||||
|
|||||||
@@ -377,7 +377,10 @@ async def _enqueue_deletes_for_disabled_types(
|
|||||||
if len(_consent_backstop_done) >= _CONSENT_BACKSTOP_MAX:
|
if len(_consent_backstop_done) >= _CONSENT_BACKSTOP_MAX:
|
||||||
# Evict oldest-first down to half capacity (insertion-ordered dict),
|
# Evict oldest-first down to half capacity (insertion-ordered dict),
|
||||||
# so overflow re-fires the backstop for only the oldest markers
|
# so overflow re-fires the backstop for only the oldest markers
|
||||||
# rather than the whole fleet at once.
|
# rather than the whole fleet at once. Placed inside the per-doc_type
|
||||||
|
# loop: markers added earlier in *this* call are the newest, so they
|
||||||
|
# survive eviction; only genuinely old entries are dropped (and a
|
||||||
|
# re-fire is idempotent regardless).
|
||||||
overage = len(_consent_backstop_done) - _CONSENT_BACKSTOP_MAX // 2
|
overage = len(_consent_backstop_done) - _CONSENT_BACKSTOP_MAX // 2
|
||||||
logger.info(
|
logger.info(
|
||||||
"consent backstop tracking hit %d entries; evicting %d oldest",
|
"consent backstop tracking hit %d entries; evicting %d oldest",
|
||||||
|
|||||||
@@ -9,8 +9,12 @@ caller skipping this helper entirely.
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
from nextcloud_mcp_server.server.semantic import _consent_narrowed_doc_types
|
from nextcloud_mcp_server.server.semantic import _consent_narrowed_doc_types
|
||||||
|
|
||||||
|
pytestmark = pytest.mark.unit
|
||||||
|
|
||||||
|
|
||||||
def test_none_request_restricts_to_allow_set():
|
def test_none_request_restricts_to_allow_set():
|
||||||
# No explicit doc_types -> search exactly the allowed set (sorted).
|
# No explicit doc_types -> search exactly the allowed set (sorted).
|
||||||
|
|||||||
@@ -4,6 +4,8 @@ from __future__ import annotations
|
|||||||
|
|
||||||
from unittest.mock import AsyncMock
|
from unittest.mock import AsyncMock
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
import nextcloud_mcp_server.capabilities as cap
|
import nextcloud_mcp_server.capabilities as cap
|
||||||
from nextcloud_mcp_server.capabilities import (
|
from nextcloud_mcp_server.capabilities import (
|
||||||
_parse_enabled_doc_types,
|
_parse_enabled_doc_types,
|
||||||
@@ -12,6 +14,8 @@ from nextcloud_mcp_server.capabilities import (
|
|||||||
is_doc_type_allowed,
|
is_doc_type_allowed,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
pytestmark = pytest.mark.unit
|
||||||
|
|
||||||
|
|
||||||
def _payload(enabled_doc_types) -> dict:
|
def _payload(enabled_doc_types) -> dict:
|
||||||
"""Build an OCS capabilities envelope carrying the astrolabe block.
|
"""Build an OCS capabilities envelope carrying the astrolabe block.
|
||||||
|
|||||||
@@ -111,6 +111,7 @@ async def test_process_document_records_drop_on_exhausted_retries(mocker):
|
|||||||
rec.assert_called_once_with("connection")
|
rec.assert_called_once_with("connection")
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
async def test_process_document_drops_admin_disabled_index_task(mocker):
|
async def test_process_document_drops_admin_disabled_index_task(mocker):
|
||||||
"""A near-real-time index task for an admin-disabled doc_type is dropped
|
"""A near-real-time index task for an admin-disabled doc_type is dropped
|
||||||
before indexing, and recorded under the ``admin_disabled`` reason."""
|
before indexing, and recorded under the ``admin_disabled`` reason."""
|
||||||
@@ -143,6 +144,7 @@ async def test_process_document_drops_admin_disabled_index_task(mocker):
|
|||||||
rec.assert_called_once_with("admin_disabled")
|
rec.assert_called_once_with("admin_disabled")
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
async def test_process_document_allows_when_doc_type_approved(mocker):
|
async def test_process_document_allows_when_doc_type_approved(mocker):
|
||||||
"""The consent gate does not drop an index task for an allowed doc_type."""
|
"""The consent gate does not drop an index task for an allowed doc_type."""
|
||||||
from nextcloud_mcp_server.vector.scanner import DocumentTask
|
from nextcloud_mcp_server.vector.scanner import DocumentTask
|
||||||
|
|||||||
@@ -168,6 +168,26 @@ def test_admin_purge_happy_path(mocker):
|
|||||||
purge.assert_awaited_once_with(["file"])
|
purge.assert_awaited_once_with(["file"])
|
||||||
|
|
||||||
|
|
||||||
|
def test_partial_failure_reports_failed_types(mocker):
|
||||||
|
# purge_doc_types returns only the succeeded types; the route must tell the
|
||||||
|
# caller which requested types were NOT purged.
|
||||||
|
_patch_token(mocker, "admin")
|
||||||
|
_patch_basic_auth(mocker, "admin")
|
||||||
|
_patch_outbound_client(mocker)
|
||||||
|
_patch_groups(mocker, ["admin"])
|
||||||
|
_patch_purge(mocker, {"file": 3}) # "note" failed
|
||||||
|
|
||||||
|
client = TestClient(_build_app())
|
||||||
|
resp = client.post(
|
||||||
|
"/api/v1/vector-sync/purge", json={"doc_types": ["file", "note"]}
|
||||||
|
)
|
||||||
|
|
||||||
|
assert resp.status_code == 200
|
||||||
|
body = resp.json()
|
||||||
|
assert body["purged"] == {"file": 3}
|
||||||
|
assert body["failed"] == ["note"]
|
||||||
|
|
||||||
|
|
||||||
def test_bad_request_when_body_not_object(mocker):
|
def test_bad_request_when_body_not_object(mocker):
|
||||||
# A valid JSON non-object (e.g. a list) must 400, not 500.
|
# A valid JSON non-object (e.g. a list) must 400, not 500.
|
||||||
_patch_token(mocker)
|
_patch_token(mocker)
|
||||||
|
|||||||
@@ -10,6 +10,8 @@ import pytest
|
|||||||
import nextcloud_mcp_server.vector.purge as purge_module
|
import nextcloud_mcp_server.vector.purge as purge_module
|
||||||
from nextcloud_mcp_server.vector.purge import purge_doc_types
|
from nextcloud_mcp_server.vector.purge import purge_doc_types
|
||||||
|
|
||||||
|
pytestmark = pytest.mark.unit
|
||||||
|
|
||||||
|
|
||||||
def _patch_qdrant(monkeypatch, *, counts: dict[str, int], delete_raises=None):
|
def _patch_qdrant(monkeypatch, *, counts: dict[str, int], delete_raises=None):
|
||||||
"""Wire a fake Qdrant client whose ``count`` reflects ``counts`` per
|
"""Wire a fake Qdrant client whose ``count`` reflects ``counts`` per
|
||||||
|
|||||||
@@ -18,6 +18,8 @@ from nextcloud_mcp_server.vector import scanner as scanner_module
|
|||||||
from nextcloud_mcp_server.vector.queue.ports import TaskProducer
|
from nextcloud_mcp_server.vector.queue.ports import TaskProducer
|
||||||
from nextcloud_mcp_server.vector.scanner import _enqueue_deletes_for_disabled_types
|
from nextcloud_mcp_server.vector.scanner import _enqueue_deletes_for_disabled_types
|
||||||
|
|
||||||
|
pytestmark = pytest.mark.unit
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture(autouse=True)
|
@pytest.fixture(autouse=True)
|
||||||
def _clear_backstop_state():
|
def _clear_backstop_state():
|
||||||
|
|||||||
Reference in New Issue
Block a user