diff --git a/nextcloud_mcp_server/vector/oauth_sync.py b/nextcloud_mcp_server/vector/oauth_sync.py index 485bdaa3..e5e6a020 100644 --- a/nextcloud_mcp_server/vector/oauth_sync.py +++ b/nextcloud_mcp_server/vector/oauth_sync.py @@ -58,9 +58,11 @@ class ProvisionSignal: event back to shared state — avoiding any ``app`` ↔ ``vector`` import cycle. Concurrency: ``anyio.Event`` is sticky, so a ``ring()`` that lands before - ``wait()`` is still observed. ``wait()`` re-arms with no ``await`` between - observing the set and swapping the event, so under cooperative scheduling a - concurrent ``ring()`` cannot slip into that window and be lost. + ``wait()`` is still observed. ``wait()`` re-arms in a ``finally`` with no + ``await`` before the swap, so under cooperative scheduling a concurrent + ``ring()`` cannot slip into that window and be lost — and the re-arm also + runs if ``wait()`` is cancelled (e.g. shutdown racing the doorbell), leaving + a fresh unset event rather than a stale set-but-consumed one. """ def __init__(self) -> None: @@ -72,10 +74,13 @@ class ProvisionSignal: async def wait(self) -> None: """Block until the next ring, then re-arm for the following cycle.""" - await self._event.wait() - # No await before the swap: a concurrent ring() cannot interleave here, - # so it lands on the fresh event and the next wait() observes it. - self._event = anyio.Event() + try: + await self._event.wait() + finally: + # Re-arm even on cancellation. The assignment is not a checkpoint, + # so no concurrent ring() can interleave before the swap; a ring + # that already arrived lands on the fresh event for the next wait(). + self._event = anyio.Event() # Process-wide app-password storage for the BasicAuth client path. @@ -475,6 +480,12 @@ async def user_manager_task( logger.info("[BasicAuth] User manager started (poll interval: %ss)", poll_interval) task_status.started() + # Sleep helper: await one of the wakeup events, then end the sleep by + # cancelling the shared scope. Defined once (not per loop iteration). + async def _wake_on(wait_fn, scope: anyio.CancelScope) -> None: + await wait_fn() + scope.cancel() + while not shutdown_event.is_set(): try: # Query the app_passwords table — background sync always @@ -537,10 +548,6 @@ async def user_manager_task( # provisioning signal so a just-provisioned user is discovered at once. # Race both waits in a child task group; whichever fires first cancels # the scope, ending the sleep. move_on_after caps it at poll_interval. - async def _wake_on(wait_fn, scope: anyio.CancelScope) -> None: - await wait_fn() - scope.cancel() - try: with anyio.move_on_after(poll_interval): async with anyio.create_task_group() as wake_tg: diff --git a/tests/unit/test_management_app_password_endpoints.py b/tests/unit/test_management_app_password_endpoints.py index 7ad924b8..80690c6b 100644 --- a/tests/unit/test_management_app_password_endpoints.py +++ b/tests/unit/test_management_app_password_endpoints.py @@ -254,6 +254,48 @@ async def test_provision_app_password_success(temp_storage, mocker): assert get_kwargs["auth"] == ("testuser", "aaaaa-bbbbb-ccccc-ddddd-eeeee") +async def test_provision_app_password_wakes_user_manager(temp_storage, mocker): + """A successful provision rings the background-sync doorbell so the user + manager re-polls immediately (the api/passwords.py wake path).""" + mocker.patch( + "nextcloud_mcp_server.api.passwords.get_settings", + return_value=MagicMock( + nextcloud_host="http://localhost:8080", + nextcloud_verify_ssl=True, + nextcloud_ca_bundle=None, + ), + ) + + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {"ocs": {"data": {"id": "testuser"}}} + mock_client = AsyncMock() + mock_client.get = AsyncMock(return_value=mock_response) + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock() + mocker.patch( + "nextcloud_mcp_server.api.passwords.nextcloud_httpx_client", + return_value=mock_client, + ) + + # Spy on the doorbell helper (imported locally from app at call time). + notify = mocker.patch("nextcloud_mcp_server.app.notify_user_provisioned") + + client = TestClient(create_test_app(temp_storage)) + response = client.post( + "/api/v1/users/testuser/app-password", + headers={ + "Authorization": create_basic_auth_header( + "testuser", "aaaaa-bbbbb-ccccc-ddddd-eeeee" + ) + }, + ) + + assert response.status_code == 200 + assert response.json()["success"] is True + notify.assert_called_once() + + async def test_provision_app_password_uses_loginname_not_uid(temp_storage, mocker): """Regression: when the Nextcloud UID differs from the loginName (e.g. OIDC-provisioned users whose UID is their display name — UID diff --git a/tests/unit/vector/test_user_manager_provision_wake.py b/tests/unit/vector/test_user_manager_provision_wake.py index d00af64b..2463f350 100644 --- a/tests/unit/vector/test_user_manager_provision_wake.py +++ b/tests/unit/vector/test_user_manager_provision_wake.py @@ -203,11 +203,13 @@ def test_notify_user_provisioned_noop_without_manager(mocker): app_module.notify_user_provisioned() -def test_notify_user_provisioned_rings_when_present(mocker): +async def test_notify_user_provisioned_rings_when_present(mocker): """When a manager is running, the helper rings its signal.""" import nextcloud_mcp_server.app as app_module signal = ProvisionSignal() mocker.patch.object(app_module._vector_sync_state, "provision_signal", signal) app_module.notify_user_provisioned() - assert signal._event.is_set() + # Public contract: after a ring, the next wait() returns without blocking. + with anyio.fail_after(1): + await signal.wait()