chore: round-4 polish + standardize on module-level loggers

Round-4 review (non-blocking) items:
- get_procrastinate_conninfo: warn on an empty connect_timeout= value (it falls
  back to the 10s default); preserve an explicit connect_timeout=0.
- Document the _doc_queueing_lock user_id invariant (NC rejects ':' in usernames).
- docs/configuration.md: note that `db downgrade` leaves procrastinate's tables
  in place and how to drop them on a full teardown.
- reclaim_stalled_ingest_jobs: debug heartbeat log when nothing is stalled.
- Drop the redundant list() wrap in the integration stalled-jobs assertion.

Logging pattern: define a module-level `logger = logging.getLogger(__name__)`
and use it instead of function-local or inline getLogger(__name__) calls
(config.py, config_validators.py, tests/.../test_scope_authorization.py). The
test file's dev-only `scripts.*` import gets a ty: ignore since it resolves via
sys.path at runtime, not as an installed package.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

docs/configuration.md

@@ -834,6 +834,13 @@ Notes:
 - KEDA scales the worker on
   `SELECT count(*) FROM procrastinate_jobs WHERE queue_name='ingest' AND status='todo'`.
 - `INGEST_QUEUE=postgres` with a SQLite `DATABASE_URL` is rejected at startup.
+- **Teardown:** because procrastinate's schema is a separate lineage,
+  `nextcloud-mcp-server db downgrade` (Alembic) does **not** drop the
+  `procrastinate_*` tables. To fully revert (e.g. back to NATS or SQLite-only),
+  drop them manually after downgrading:
+  `DROP TABLE IF EXISTS procrastinate_jobs, procrastinate_events,
+  procrastinate_periodic_defers, procrastinate_workers CASCADE;` (plus the
+  `procrastinate_*` types/functions if removing the extension entirely).
 
 ---
 

nextcloud_mcp_server/config.py

@@ -11,6 +11,8 @@ from typing import Any
 
 from dynaconf import Dynaconf, Validator
 
+logger = logging.getLogger(__name__)
+
 # Sentinel for "key not in dynaconf at all" vs "explicitly set to None".
 _UNSET = object()
 
@@ -734,7 +736,6 @@ class Settings:
 
     def __post_init__(self):
         """Validate configuration and set defaults."""
-        logger = logging.getLogger(__name__)
 
         # Validate SSL/TLS configuration
         if not self.nextcloud_verify_ssl:
@@ -1064,8 +1065,6 @@ def _get_semantic_search_enabled() -> bool:
     Returns:
         True if semantic search should be enabled
     """
-    logger = logging.getLogger(__name__)
-
     new_value = _dynaconf.get("ENABLE_SEMANTIC_SEARCH", False)
     old_value = _dynaconf.get("VECTOR_SYNC_ENABLED", False)
 
@@ -1146,7 +1145,6 @@ def _log_bg_ops_advisories_once(
         return
     _bg_ops_advisories_logged = True
 
-    logger = logging.getLogger(__name__)
     if explicit and legacy:
         logger.warning(
             "Both ENABLE_BACKGROUND_OPERATIONS and ENABLE_OFFLINE_ACCESS are set. "
@@ -1473,7 +1471,7 @@ def get_procrastinate_conninfo(database_url: str | None = None) -> str:
     # query string is dropped with a warning.
     dropped = sorted(k for k in url.query if k != "connect_timeout")
     if dropped:
-        logging.getLogger(__name__).warning(
+        logger.warning(
             "Dropping DATABASE_URL query parameters not forwarded to the "
             "procrastinate connector: %s",
             ", ".join(dropped),
@@ -1492,10 +1490,14 @@ def get_procrastinate_conninfo(database_url: str | None = None) -> str:
         params["password"] = url.password
     # Honor an operator-supplied connect_timeout, else default to 10s. (make_url
     # query values are str or a tuple of strs when repeated; take the last.)
+    # ``connect_timeout=0`` (disable) is preserved — only a missing/empty value
+    # falls back to the default, and an explicit-but-empty value is flagged.
     _ct = url.query.get("connect_timeout")
     if isinstance(_ct, (list, tuple)):
         _ct = _ct[-1] if _ct else None
-    params["connect_timeout"] = str(_ct) if _ct else "10"
+    if _ct == "":
+        logger.warning("DATABASE_URL has an empty connect_timeout=; using default 10s")
+    params["connect_timeout"] = _ct if _ct else "10"
     params.update(_pg_ssl_params())
 
     return make_conninfo(**params)

nextcloud_mcp_server/config_validators.py

@@ -187,8 +187,6 @@ def detect_auth_mode(settings: Settings) -> AuthMode:
         ValueError: If explicit deployment_mode is unrecognised.
     """
 
-    logger = logging.getLogger(__name__)
-
     # ADR-021: explicit deployment mode wins
     if settings.deployment_mode:
         mode_str = settings.deployment_mode.lower().strip()

nextcloud_mcp_server/vector/queue/procrastinate.py

@@ -136,6 +136,10 @@ async def reclaim_stalled_ingest_jobs(context: JobContext, timestamp: int) -> No
         reclaimed += 1
     if reclaimed:
         logger.warning("ingest.reclaimed_stalled_jobs count=%d", reclaimed)
+    else:
+        # Visible heartbeat under verbose logging when debugging a suspected
+        # reclaim failure, without noising up production logs.
+        logger.debug("ingest.reclaim_check stalled=0")
 
 
 async def _resolve_client(user_id: str) -> NextcloudClient:
@@ -293,11 +297,16 @@ def _doc_queueing_lock(task: DocumentTask) -> str:
     """Per-document enqueue-dedup key (partial-unique on ``status='todo'``).
 
     Collision-safe with a raw ``:`` delimiter because the first two segments can
-    never themselves contain ``:``: ``user_id`` is a Nextcloud username/UID (no
-    colons) and ``doc_type`` is a controlled enum (``note``/``file``/
-    ``deck_card``/``news_item``). The trailing ``doc_id`` may contain anything —
-    it's the final unambiguous segment. A future ``doc_type`` containing ``:``
-    would break this invariant, so keep doc_type colon-free.
+    never themselves contain ``:``:
+
+    - ``user_id`` — a Nextcloud username/UID; Nextcloud rejects ``:`` in
+      usernames (Web UI + provisioning validation), so it is colon-free.
+    - ``doc_type`` — a controlled enum (``note``/``file``/``deck_card``/
+      ``news_item``); a future ``doc_type`` containing ``:`` would break this
+      invariant, so keep doc_type colon-free.
+
+    The trailing ``doc_id`` may contain anything — it's the final unambiguous
+    segment.
     """
     return f"{task.user_id}:{task.doc_type}:{task.doc_id}"
 

tests/integration/test_ingest_queue_postgres.py

@@ -133,9 +133,7 @@ async def test_ingest_queue_end_to_end(fresh_app):
         assert counts.get("todo") == 2
 
         # 4. Fresh todo jobs are not "doing", so none are stalled.
-        stalled = list(
-            await fresh_app.job_manager.get_stalled_jobs(
-                queue=INGEST_QUEUE_NAME, seconds_since_heartbeat=0
-            )
+        stalled = await fresh_app.job_manager.get_stalled_jobs(
+            queue=INGEST_QUEUE_NAME, seconds_since_heartbeat=0
         )
-        assert stalled == []
+        assert list(stalled) == []

tests/server/login_flow/test_scope_authorization.py

@@ -16,6 +16,8 @@ import logging
 import httpx
 import pytest
 
+logger = logging.getLogger(__name__)
+
 
 @pytest.mark.integration
 @pytest.mark.login_flow
@@ -65,8 +67,6 @@ async def test_basicauth_shows_all_tools(nc_mcp_client):
 async def test_read_only_token_filters_write_tools(nc_mcp_login_flow_client_read_only):
     """Test that a token with only read scopes filters out write tools."""
 
-    logger = logging.getLogger(__name__)
-
     # Connect with token that has only "notes.read" scope
     result = await nc_mcp_login_flow_client_read_only.list_tools()
     assert result is not None
@@ -114,8 +114,6 @@ async def test_read_only_token_filters_write_tools(nc_mcp_login_flow_client_read
 async def test_write_only_token_filters_read_tools(nc_mcp_login_flow_client_write_only):
     """Test that a token with only write scopes filters out read tools."""
 
-    logger = logging.getLogger(__name__)
-
     # Connect with token that has only "notes.write" scope
     result = await nc_mcp_login_flow_client_write_only.list_tools()
     assert result is not None
@@ -163,8 +161,6 @@ async def test_write_only_token_filters_read_tools(nc_mcp_login_flow_client_writ
 async def test_full_access_token_shows_all_tools(nc_mcp_login_flow_client_full_access):
     """Test that a token with both read and write scopes scopes can see all tools."""
 
-    logger = logging.getLogger(__name__)
-
     # Connect with token that has both "notes.read" and "notes.write" scopes
     result = await nc_mcp_login_flow_client_full_access.list_tools()
     assert result is not None
@@ -305,7 +301,11 @@ async def test_tools_have_scope_decorators(nc_mcp_client):
 @pytest.mark.integration
 async def test_scope_classification():
     """Test that our scope classification correctly identifies read vs write operations."""
-    from scripts.add_scope_decorators_simple import classify_function
+    # `scripts/` is a dev-only helper dir (not an installed package); resolved
+    # at runtime via the repo root on sys.path, so ty can't see it.
+    from scripts.add_scope_decorators_simple import (  # ty: ignore[unresolved-import]
+        classify_function,
+    )
 
     # Test read operations
     assert classify_function("nc_notes_get_note") == "notes.read"
@@ -336,7 +336,11 @@ async def test_scope_classification():
 @pytest.mark.integration
 async def test_all_tools_classified():
     """Verify that all tools can be properly classified as read or write."""
-    from scripts.add_scope_decorators_simple import classify_function
+    # `scripts/` is a dev-only helper dir (not an installed package); resolved
+    # at runtime via the repo root on sys.path, so ty can't see it.
+    from scripts.add_scope_decorators_simple import (  # ty: ignore[unresolved-import]
+        classify_function,
+    )
 
     # List of all tool names (extracted from our implementation)
     all_tools = [
@@ -407,8 +411,6 @@ async def test_jwt_with_no_custom_scopes_returns_zero_tools(
       so users can provision Nextcloud access after authentication
     """
 
-    logger = logging.getLogger(__name__)
-
     # Connect with JWT token that has NO custom scopes (only openid, profile, email)
     result = await nc_mcp_login_flow_client_no_custom_scopes.list_tools()
     assert result is not None
@@ -451,8 +453,6 @@ async def test_jwt_consent_scenarios_read_only(nc_mcp_login_flow_client_read_onl
     Expected: Should see read tools but not write tools.
     """
 
-    logger = logging.getLogger(__name__)
-
     result = await nc_mcp_login_flow_client_read_only.list_tools()
     assert result is not None
     assert len(result.tools) > 0
@@ -490,8 +490,6 @@ async def test_jwt_consent_scenarios_write_only(nc_mcp_login_flow_client_write_o
     Expected: Should see write tools but not read-only tools.
     """
 
-    logger = logging.getLogger(__name__)
-
     result = await nc_mcp_login_flow_client_write_only.list_tools()
     assert result is not None
     assert len(result.tools) > 0
@@ -529,8 +527,6 @@ async def test_jwt_consent_scenarios_full_access(nc_mcp_login_flow_client_full_a
     Expected: Should see all 90+ tools (both read and write).
     """
 
-    logger = logging.getLogger(__name__)
-
     result = await nc_mcp_login_flow_client_full_access.list_tools()
     assert result is not None
     assert len(result.tools) > 0