Merge remote-tracking branch 'origin/master' into fix/glyph-corruption-structured-escalation

CHANGELOG.md

@@ -5,6 +5,13 @@ All notable changes to the Nextcloud MCP Server will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [PEP 440](https://peps.python.org/pep-0440/).
 
+## v0.120.1 (2026-06-16)
+
+### Fix
+
+- **vector**: propagate cancel in cleanup task; cover 403 + sweep-failure
+- **vector**: self-heal stale app passwords on auth failure
+
 ## v0.120.0 (2026-06-16)
 
 ### Feat

nextcloud_mcp_server/app.py

@@ -130,6 +130,7 @@ from nextcloud_mcp_server.server.oauth_tools import register_oauth_tools
 from nextcloud_mcp_server.vector.metrics_publisher import vector_sync_metrics_task
 from nextcloud_mcp_server.vector.oauth_sync import (
     ProvisionSignal,
+    credential_cleanup_task,
     oauth_processor_task,
     user_manager_task,
 )
@@ -2067,9 +2068,23 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None =
                 # how many per-user scanners the user-manager later starts.
                 await _sweep_orphan_placeholders_if_enabled()
 
-                # Clean up stale app passwords at startup (BasicAuth mode only)
+                # Clean up stale app passwords at startup. All deployment modes
-                if not oauth_enabled:
+                # now authenticate background sync via locally-stored app
+                # passwords (the OAuth refresh-token path was removed), so this
+                # must run regardless of oauth_enabled — login_flow tenants were
+                # previously skipped, letting deleted-user credentials linger and
+                # drive an endless scanner re-spawn/401 loop (Deck #198). The
+                # credential_cleanup_task started below repeats it on a cadence.
                 try:
+                    # Log the cohort first: the sweep makes one OCS validation
+                    # call per stored user before readiness, so the count is the
+                    # operability signal if startup latency ever climbs.
+                    stored = await token_storage.get_all_app_password_user_ids()
+                    if stored:
+                        logger.info(
+                            "Running startup credential sweep for %s stored user(s)",
+                            len(stored),
+                        )
                     removed = await token_storage.cleanup_invalid_app_passwords(
                         nextcloud_host=nextcloud_host_for_sync
                     )
@@ -2134,6 +2149,16 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None =
                         provision_signal,
                     )
 
+                    # Periodic backstop sweep removing app passwords that no
+                    # longer authenticate (deleted/disabled users), complementing
+                    # the per-scanner self-heal in user_scanner_task (Deck #198).
+                    await tg.start(
+                        credential_cleanup_task,
+                        token_storage,
+                        shutdown_event,
+                        nextcloud_host_for_sync,
+                    )
+
                     # In-process consumer pool. ``run_consumers`` is a no-op for
                     # the distributed (postgres) backend — the out-of-process
                     # ``worker`` role consumes there. The closure binds this

nextcloud_mcp_server/vector/oauth_sync.py

@@ -125,6 +125,47 @@ class UserSyncState:
     started_at: float = field(default_factory=time.time)
 
 
+async def _remove_stale_credential(user_id: str, status_code: int) -> None:
+    """Delete a user's stored app password after a hard auth failure.
+
+    A 401/403 from Nextcloud means the stored app password is no longer usable —
+    the user was deleted, disabled, or revoked the password. Without removing it
+    the credential lingers in storage and ``user_manager_task`` re-spawns this
+    scanner on its next poll (the user is still "provisioned"), only to fail auth
+    again — an endless re-spawn/401 loop that keeps hammering Nextcloud (Deck
+    #198). Deleting the row drops the user out of
+    ``get_all_app_password_user_ids()`` so the scanner is not recreated.
+
+    Convergence: if ``user_manager_task`` already snapshotted the user IDs for
+    the current poll before this deletion, it re-spawns the scanner once more on
+    the next cycle, which fails auth and deletes again — at most one extra 401
+    per manager poll interval, versus the unbounded loop before this fix.
+
+    Best-effort: a storage failure here is logged, not raised — the periodic
+    ``credential_cleanup_task`` sweep (and the next startup sweep) are backstops.
+    """
+    try:
+        storage = await _get_initialized_basic_auth_storage()
+        if await storage.delete_app_password(user_id):
+            logger.info(
+                "[BasicAuth] Removed stale app password for %s after HTTP %s",
+                user_id,
+                status_code,
+            )
+        else:
+            # Row already gone — raced with the periodic sweep or another
+            # scanner exit. Harmless; logged for diagnostics.
+            logger.debug(
+                "[BasicAuth] No stale app password to remove for %s (HTTP %s)",
+                user_id,
+                status_code,
+            )
+    except Exception as e:
+        logger.warning(
+            "[BasicAuth] Failed to remove stale app password for %s: %s", user_id, e
+        )
+
+
 async def get_user_client_basic_auth(
     user_id: str,
     nextcloud_host: str,
@@ -215,10 +256,12 @@ async def user_scanner_task(
         except HTTPStatusError as e:
             if e.response.status_code in (401, 403):
                 logger.warning(
-                    "[BasicAuth] Credential validation failed for %s (HTTP %s), not starting scan loop",
+                    "[BasicAuth] Credential validation failed for %s (HTTP %s), "
+                    "removing stale credential and not starting scan loop",
                     user_id,
                     e.response.status_code,
                 )
+                await _remove_stale_credential(user_id, e.response.status_code)
                 return
             raise
         finally:
@@ -262,10 +305,12 @@ async def user_scanner_task(
             status_code = e.response.status_code
             if status_code in (401, 403):
                 logger.warning(
-                    "[BasicAuth] Scanner auth failed for %s (HTTP %s), stopping scanner. User may need to re-provision credentials.",
+                    "[BasicAuth] Scanner auth failed for %s (HTTP %s), removing stale "
+                    "credential and stopping scanner. User must re-provision to resume sync.",
                     user_id,
                     status_code,
                 )
+                await _remove_stale_credential(user_id, status_code)
                 break
             elif status_code == 429:
                 retry_after = min(int(e.response.headers.get("Retry-After", "60")), 300)
@@ -410,6 +455,64 @@ async def multi_user_processor_task(
 oauth_processor_task = multi_user_processor_task
 
 
+# Backstop sweep cadence for cleanup_invalid_app_passwords (seconds). The
+# per-scanner 401 deletion (_remove_stale_credential) is the primary self-heal;
+# this periodic sweep is defense-in-depth for credentials whose scanner never
+# ran or whose in-scanner deletion failed. One hour keeps the per-user OCS
+# validation load negligible.
+CREDENTIAL_CLEANUP_INTERVAL = 3600
+
+
+async def credential_cleanup_task(
+    storage: "RefreshTokenStorage",
+    shutdown_event: anyio.Event,
+    nextcloud_host: str,
+    *,
+    task_status: TaskStatus = anyio.TASK_STATUS_IGNORED,
+) -> None:
+    """Periodically remove app passwords that no longer authenticate.
+
+    Backstop for the per-scanner self-heal (``_remove_stale_credential``):
+    validates every stored app password against Nextcloud and deletes the ones
+    that return 401/403, so credentials for deleted/disabled users cannot
+    accumulate even if their scanner never ran (Deck #198). Runs on a fixed
+    cadence until ``shutdown_event`` is set. A fresh sweep already runs once at
+    startup (app lifespan), so this sleeps first.
+    """
+    logger.info(
+        "[BasicAuth] Credential cleanup task started (interval: %ss)",
+        CREDENTIAL_CLEANUP_INTERVAL,
+    )
+    task_status.started()
+
+    while not shutdown_event.is_set():
+        # Sleep first — startup already swept; wake early on shutdown. The
+        # graceful path goes through shutdown_event (move_on_after returns once
+        # teardown sets it), so we deliberately do NOT catch the cancellation
+        # exception: a task-group cancel must propagate for structured-
+        # concurrency teardown (re-swallowing it would breach anyio's contract).
+        with anyio.move_on_after(CREDENTIAL_CLEANUP_INTERVAL):
+            await shutdown_event.wait()
+        if shutdown_event.is_set():
+            break
+
+        try:
+            removed = await storage.cleanup_invalid_app_passwords(nextcloud_host)
+            if removed:
+                logger.info(
+                    "[BasicAuth] Periodic cleanup removed %s stale app password(s): %s",
+                    len(removed),
+                    removed,
+                )
+        except Exception as e:
+            logger.warning(
+                "[BasicAuth] Periodic credential cleanup failed (non-fatal): %s",
+                format_exception_group(e),
+            )
+
+    logger.info("[BasicAuth] Credential cleanup task stopped")
+
+
 async def _run_user_scanner_with_scope(
     user_id: str,
     cancel_scope: anyio.CancelScope,

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "nextcloud-mcp-server"
-version = "0.120.0"
+version = "0.120.1"
 description = "Model Context Protocol (MCP) server for Nextcloud integration - enables AI assistants to interact with Nextcloud data"
 authors = [
     {name = "Chris Coutinho", email = "chris@coutinho.io"}

tests/unit/vector/test_credential_self_heal.py

@@ -0,0 +1,168 @@
+"""Unit tests: self-healing removal of stale app passwords (Deck #198).
+
+When a Nextcloud user is deleted/disabled, their stored app password keeps
+returning 401/403. Previously the scanner just stopped, leaving the credential in
+storage so ``user_manager_task`` re-spawned the scanner every poll interval — an
+endless re-spawn/401 loop. The scanner now deletes the credential on a hard auth
+failure, and a periodic ``credential_cleanup_task`` sweeps any that slip through.
+"""
+
+import logging
+
+import anyio
+import httpx
+import pytest
+
+from nextcloud_mcp_server.vector import oauth_sync
+
+pytestmark = pytest.mark.unit
+
+
+def _http_error(status: int) -> httpx.HTTPStatusError:
+    req = httpx.Request("GET", "https://cloud.example.org/ocs")
+    return httpx.HTTPStatusError(
+        "auth failure", request=req, response=httpx.Response(status, request=req)
+    )
+
+
+async def test_remove_stale_credential_deletes(mocker):
+    """The helper deletes the user's app password via storage."""
+    storage = mocker.MagicMock()
+    storage.delete_app_password = mocker.AsyncMock(return_value=True)
+    mocker.patch.object(
+        oauth_sync,
+        "_get_initialized_basic_auth_storage",
+        mocker.AsyncMock(return_value=storage),
+    )
+
+    await oauth_sync._remove_stale_credential("ghost-user", 401)
+
+    storage.delete_app_password.assert_awaited_once_with("ghost-user")
+
+
+async def test_remove_stale_credential_swallows_storage_error(mocker):
+    """Best-effort: a storage failure is logged, never raised (backstops cover it)."""
+    storage = mocker.MagicMock()
+    storage.delete_app_password = mocker.AsyncMock(side_effect=RuntimeError("db down"))
+    mocker.patch.object(
+        oauth_sync,
+        "_get_initialized_basic_auth_storage",
+        mocker.AsyncMock(return_value=storage),
+    )
+
+    # Must not raise.
+    await oauth_sync._remove_stale_credential("ghost-user", 401)
+
+
+@pytest.mark.parametrize("status", [401, 403])
+async def test_scanner_removes_credential_on_prevalidation_auth_failure(mocker, status):
+    """A 401/403 validating creds deletes the credential and never enters the scan
+    loop — so ``user_manager_task`` won't see the user as provisioned again."""
+    fake_client = mocker.AsyncMock()
+    fake_client.capabilities = mocker.AsyncMock(side_effect=_http_error(status))
+    fake_client.close = mocker.AsyncMock()
+    mocker.patch.object(
+        oauth_sync,
+        "get_user_client_basic_auth",
+        mocker.AsyncMock(return_value=fake_client),
+    )
+    storage = mocker.MagicMock()
+    storage.delete_app_password = mocker.AsyncMock(return_value=True)
+    mocker.patch.object(
+        oauth_sync,
+        "_get_initialized_basic_auth_storage",
+        mocker.AsyncMock(return_value=storage),
+    )
+
+    await oauth_sync.user_scanner_task(
+        "ghost-user",
+        mocker.MagicMock(),  # send_stream — unused on the pre-validation path
+        anyio.Event(),  # shutdown_event
+        anyio.Event(),  # wake_event
+        "https://cloud.example.org",
+    )
+
+    storage.delete_app_password.assert_awaited_once_with("ghost-user")
+
+
+@pytest.mark.parametrize("status", [401, 403])
+async def test_scanner_removes_credential_on_scan_loop_auth_failure(mocker, status):
+    """A 401/403 raised while scanning (not pre-validation) also deletes the
+    credential before the scanner stops."""
+    fake_client = mocker.AsyncMock()
+    fake_client.capabilities = mocker.AsyncMock(return_value={})  # pre-validation ok
+    fake_client.close = mocker.AsyncMock()
+    mocker.patch.object(
+        oauth_sync,
+        "get_user_client_basic_auth",
+        mocker.AsyncMock(return_value=fake_client),
+    )
+    mocker.patch.object(
+        oauth_sync,
+        "scan_user_documents",
+        mocker.AsyncMock(side_effect=_http_error(status)),
+    )
+    storage = mocker.MagicMock()
+    storage.delete_app_password = mocker.AsyncMock(return_value=True)
+    mocker.patch.object(
+        oauth_sync,
+        "_get_initialized_basic_auth_storage",
+        mocker.AsyncMock(return_value=storage),
+    )
+
+    await oauth_sync.user_scanner_task(
+        "ghost-user",
+        mocker.MagicMock(),
+        anyio.Event(),
+        anyio.Event(),
+        "https://cloud.example.org",
+    )
+
+    storage.delete_app_password.assert_awaited_once_with("ghost-user")
+
+
+async def test_credential_cleanup_task_sweeps_then_stops(mocker):
+    """The periodic backstop validates stored passwords via
+    ``cleanup_invalid_app_passwords`` and exits on shutdown."""
+    mocker.patch.object(oauth_sync, "CREDENTIAL_CLEANUP_INTERVAL", 0)
+    shutdown = anyio.Event()
+    storage = mocker.MagicMock()
+
+    async def _cleanup(host):
+        shutdown.set()  # stop the loop after the first sweep
+        return ["ghost-user"]
+
+    storage.cleanup_invalid_app_passwords = mocker.AsyncMock(side_effect=_cleanup)
+
+    await oauth_sync.credential_cleanup_task(
+        storage, shutdown, "https://cloud.example.org"
+    )
+
+    storage.cleanup_invalid_app_passwords.assert_awaited_once_with(
+        "https://cloud.example.org"
+    )
+
+
+async def test_credential_cleanup_task_swallows_sweep_exception(mocker, caplog):
+    """A failing sweep is logged non-fatally and does not crash the task — the
+    loop survives to retry on the next cadence."""
+    mocker.patch.object(oauth_sync, "CREDENTIAL_CLEANUP_INTERVAL", 0)
+    shutdown = anyio.Event()
+    storage = mocker.MagicMock()
+
+    async def _boom(host):
+        shutdown.set()  # exit after this (failed) iteration
+        raise RuntimeError("sweep failed")
+
+    storage.cleanup_invalid_app_passwords = mocker.AsyncMock(side_effect=_boom)
+
+    with caplog.at_level(
+        logging.WARNING, logger="nextcloud_mcp_server.vector.oauth_sync"
+    ):
+        # Must return normally — the exception is swallowed, not propagated.
+        await oauth_sync.credential_cleanup_task(
+            storage, shutdown, "https://cloud.example.org"
+        )
+
+    storage.cleanup_invalid_app_passwords.assert_awaited_once()
+    assert "non-fatal" in caplog.text

uv.lock

@@ -2183,7 +2183,7 @@ wheels = [
 
 [[package]]
 name = "nextcloud-mcp-server"
-version = "0.120.0"
+version = "0.120.1"
 source = { editable = "." }
 dependencies = [
     { name = "aiosqlite" },