fix: address PR review feedback and fix CI test failures

- Revert default transport to streamable-http (not a breaking change) - Extract AVAILABLE_APPS constant to server/__init__.py (DRY) - Wrap get_stdio_mcp ValueError in click.ClickException for clean errors - Fix test_stdio.py: call _reload_config() so dynaconf sees env changes - Use lazy %-style logging in stdio.py - Add private API comments in test assertions - Derive --enable-app CLI choices from AVAILABLE_APPS - README: show explicit --transport stdio in uvx examples Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-07 23:03:12 +02:00
parent 09006fcea9
commit e9c46a04a0
8 changed files with 56 additions and 90 deletions
@@ -27,4 +27,4 @@ ENV VIRTUAL_ENV=/app/.venv
 ENV PATH=/app/.venv/bin:$PATH
 ENV TESSDATA_PREFIX=/usr/share/tesseract-ocr/5/tessdata

-ENTRYPOINT ["/app/.venv/bin/nextcloud-mcp-server", "run", "--transport", "streamable-http", "--host", "0.0.0.0"]
+ENTRYPOINT ["/app/.venv/bin/nextcloud-mcp-server", "run", "--host", "0.0.0.0"]
@@ -23,7 +23,7 @@ Run the server locally with [uvx](https://docs.astral.sh/uv/) (no installation r
 NEXTCLOUD_HOST=https://your.nextcloud.instance.com \
 NEXTCLOUD_USERNAME=your_username \
 NEXTCLOUD_PASSWORD=your_app_password \
-  uvx nextcloud-mcp-server run
+  uvx nextcloud-mcp-server run --transport stdio
 ```

 Or add it directly to your MCP client configuration (e.g. `claude_desktop_config.json` or `.claude/settings.json`):
@@ -33,7 +33,7 @@ Or add it directly to your MCP client configuration (e.g. `claude_desktop_config
  "mcpServers": {
    "nextcloud": {
      "command": "uvx",
-      "args": ["nextcloud-mcp-server", "run"],
+      "args": ["nextcloud-mcp-server", "run", "--transport", "stdio"],
      "env": {
        "NEXTCLOUD_HOST": "https://your.nextcloud.instance.com",
        "NEXTCLOUD_USERNAME": "your_username",
@@ -119,17 +119,8 @@ from nextcloud_mcp_server.observability.metrics import (
    set_dependency_health,
 )
 from nextcloud_mcp_server.server import (
-    configure_calendar_tools,
-    configure_collectives_tools,
-    configure_contacts_tools,
-    configure_cookbook_tools,
-    configure_deck_tools,
-    configure_news_tools,
-    configure_notes_tools,
+    AVAILABLE_APPS,
    configure_semantic_tools,
-    configure_sharing_tools,
-    configure_tables_tools,
-    configure_webdav_tools,
 )
 from nextcloud_mcp_server.server.auth_tools import register_auth_tools
 from nextcloud_mcp_server.server.oauth_tools import register_oauth_tools
@@ -1232,32 +1223,18 @@ def get_app(transport: str = "streamable-http", enabled_apps: list[str] | None =
        client = await get_nextcloud_client(ctx)
        return await client.capabilities()

-    # Define available apps and their configuration functions
-    available_apps = {
-        "notes": configure_notes_tools,
-        "tables": configure_tables_tools,
-        "webdav": configure_webdav_tools,
-        "sharing": configure_sharing_tools,
-        "calendar": configure_calendar_tools,
-        "collectives": configure_collectives_tools,
-        "contacts": configure_contacts_tools,
-        "cookbook": configure_cookbook_tools,
-        "deck": configure_deck_tools,
-        "news": configure_news_tools,
-    }
-
    # If no specific apps are specified, enable all
    if enabled_apps is None:
-        enabled_apps = list(available_apps.keys())
+        enabled_apps = list(AVAILABLE_APPS.keys())

    # Configure only the enabled apps
    for app_name in enabled_apps:
-        if app_name in available_apps:
+        if app_name in AVAILABLE_APPS:
            logger.info(f"Configuring {app_name} tools")
-            available_apps[app_name](mcp)
+            AVAILABLE_APPS[app_name](mcp)
        else:
            logger.warning(
-                f"Unknown app: {app_name}. Available apps: {list(available_apps.keys())}"
+                f"Unknown app: {app_name}. Available apps: {list(AVAILABLE_APPS.keys())}"
            )

    # Register semantic search tools (cross-app feature)
@@ -14,6 +14,7 @@ from nextcloud_mcp_server.migrations import (
    upgrade_database,
 )
 from nextcloud_mcp_server.observability import get_uvicorn_logging_config
+from nextcloud_mcp_server.server import AVAILABLE_APPS

 from .app import get_app

@@ -36,29 +37,16 @@ from .app import get_app
@click.option(
    "--transport",
    "-t",
-    default="stdio",
+    default="streamable-http",
    show_default=True,
-    type=click.Choice(["stdio", "streamable-http", "http"]),
+    type=click.Choice(["streamable-http", "http", "stdio"]),
    help="MCP transport protocol",
 )
@click.option(
    "--enable-app",
    "-e",
    multiple=True,
-    type=click.Choice(
-        [
-            "notes",
-            "tables",
-            "webdav",
-            "calendar",
-            "contacts",
-            "cookbook",
-            "deck",
-            "news",
-            "collectives",
-            "sharing",
-        ]
-    ),
+    type=click.Choice(sorted(AVAILABLE_APPS.keys())),
    help="Enable specific Nextcloud app APIs. Can be specified multiple times. If not specified, all apps are enabled.",
 )
@click.option(
@@ -264,7 +252,10 @@ def run(
            )
        from .stdio import get_stdio_mcp  # noqa: PLC0415

-        mcp = get_stdio_mcp(enabled_apps=enabled_apps)
+        try:
+            mcp = get_stdio_mcp(enabled_apps=enabled_apps)
+        except ValueError as e:
+            raise click.ClickException(str(e)) from e
        mcp.run(transport="stdio")
        return

@@ -1,3 +1,5 @@
+from typing import Callable
+
 from .calendar import configure_calendar_tools
 from .collectives import configure_collectives_tools
 from .contacts import configure_contacts_tools
@@ -10,7 +12,25 @@ from .sharing import configure_sharing_tools
 from .tables import configure_tables_tools
 from .webdav import configure_webdav_tools

+# Canonical mapping of app name → tool registration function.
+# Used by app.py (HTTP), stdio.py (stdio), and cli.py (--enable-app choices).
+# Semantic search is excluded here because it is a cross-app feature gated
+# by VECTOR_SYNC_ENABLED, not an individual Nextcloud app.
+AVAILABLE_APPS: dict[str, Callable] = {
+    "notes": configure_notes_tools,
+    "tables": configure_tables_tools,
+    "webdav": configure_webdav_tools,
+    "sharing": configure_sharing_tools,
+    "calendar": configure_calendar_tools,
+    "collectives": configure_collectives_tools,
+    "contacts": configure_contacts_tools,
+    "cookbook": configure_cookbook_tools,
+    "deck": configure_deck_tools,
+    "news": configure_news_tools,
+}
+
 __all__ = [
+    "AVAILABLE_APPS",
    "configure_calendar_tools",
    "configure_collectives_tools",
    "configure_contacts_tools",
@@ -11,7 +11,6 @@ import logging
 from collections.abc import AsyncIterator
 from contextlib import asynccontextmanager
 from dataclasses import dataclass
-from typing import Callable

 from mcp.server.fastmcp import Context, FastMCP

@@ -19,18 +18,7 @@ from nextcloud_mcp_server.client import NextcloudClient
 from nextcloud_mcp_server.config import get_settings
 from nextcloud_mcp_server.config_validators import AuthMode, validate_configuration
 from nextcloud_mcp_server.context import get_client as get_nextcloud_client
-from nextcloud_mcp_server.server import (
-    configure_calendar_tools,
-    configure_collectives_tools,
-    configure_contacts_tools,
-    configure_cookbook_tools,
-    configure_deck_tools,
-    configure_news_tools,
-    configure_notes_tools,
-    configure_sharing_tools,
-    configure_tables_tools,
-    configure_webdav_tools,
-)
+from nextcloud_mcp_server.server import AVAILABLE_APPS

 logger = logging.getLogger(__name__)

@@ -99,30 +87,18 @@ def get_stdio_mcp(enabled_apps: list[str] | None = None) -> FastMCP:
        return await client.capabilities()

    # --- tool registration ---
-    available_apps: dict[str, Callable] = {
-        "notes": configure_notes_tools,
-        "tables": configure_tables_tools,
-        "webdav": configure_webdav_tools,
-        "sharing": configure_sharing_tools,
-        "calendar": configure_calendar_tools,
-        "collectives": configure_collectives_tools,
-        "contacts": configure_contacts_tools,
-        "cookbook": configure_cookbook_tools,
-        "deck": configure_deck_tools,
-        "news": configure_news_tools,
-    }
-
    if enabled_apps is None:
-        enabled_apps = list(available_apps.keys())
+        enabled_apps = list(AVAILABLE_APPS.keys())

    for app_name in enabled_apps:
-        if app_name in available_apps:
-            logger.info(f"Configuring {app_name} tools")
-            available_apps[app_name](mcp)
+        if app_name in AVAILABLE_APPS:
+            logger.info("Configuring %s tools", app_name)
+            AVAILABLE_APPS[app_name](mcp)
        else:
            logger.warning(
-                f"Unknown app: {app_name}. "
-                f"Available apps: {list(available_apps.keys())}"
+                "Unknown app: %s. Available apps: %s",
+                app_name,
+                list(AVAILABLE_APPS.keys()),
            )

    return mcp
@@ -108,8 +108,6 @@ def test_cli_options_set_environment_variables(runner, clean_env, monkeypatch):
    _ = runner.invoke(
        run,
        [
-            "--transport",
-            "streamable-http",
            "--nextcloud-host",
            "https://test.example.com",
            "--nextcloud-username",
@@ -166,8 +164,6 @@ def test_cli_options_override_environment_variables(runner, monkeypatch):
    _ = runner.invoke(
        run,
        [
-            "--transport",
-            "streamable-http",
            "--nextcloud-host",
            "https://from-cli.example.com",
            "--nextcloud-username",
@@ -218,7 +214,7 @@ def test_environment_variables_used_when_cli_not_provided(runner, monkeypatch):
    monkeypatch.setattr("nextcloud_mcp_server.cli.get_app", mock_get_app)

    # Don't provide any CLI options - should use env vars
-    _ = runner.invoke(run, ["--transport", "streamable-http"])
+    _ = runner.invoke(run, [])

    # Verify env vars were used
    assert captured_env["NEXTCLOUD_HOST"] == "https://from-env.example.com"
@@ -250,7 +246,7 @@ def test_default_values(runner, clean_env, monkeypatch):
    monkeypatch.setattr("nextcloud_mcp_server.cli.get_app", mock_get_app)

    # Don't provide CLI options or env vars - should use defaults
-    _ = runner.invoke(run, ["--transport", "streamable-http"])
+    _ = runner.invoke(run, [])

    # Verify default values
    assert captured_env["NEXTCLOUD_OIDC_SCOPES"] == (
@@ -282,14 +278,12 @@ def test_oauth_token_type_case_normalization(runner, clean_env, monkeypatch):
    monkeypatch.setattr("nextcloud_mcp_server.cli.get_app", mock_get_app)

    # Test uppercase JWT
-    runner.invoke(run, ["--transport", "streamable-http", "--oauth-token-type", "JWT"])
+    runner.invoke(run, ["--oauth-token-type", "JWT"])
    assert captured_env["NEXTCLOUD_OIDC_TOKEN_TYPE"] in ["JWT", "jwt"]

    # Test mixed case Bearer
    captured_env.clear()
-    runner.invoke(
-        run, ["--transport", "streamable-http", "--oauth-token-type", "Bearer"]
-    )
+    runner.invoke(run, ["--oauth-token-type", "Bearer"])
    assert captured_env["NEXTCLOUD_OIDC_TOKEN_TYPE"] in ["Bearer", "bearer"]


@@ -3,6 +3,7 @@
 import pytest
 from mcp.server.fastmcp import FastMCP

+from nextcloud_mcp_server.config import _reload_config
 from nextcloud_mcp_server.config_validators import AuthMode
 from nextcloud_mcp_server.stdio import get_stdio_mcp

@@ -13,6 +14,9 @@ def single_user_env(monkeypatch):
    monkeypatch.setenv("NEXTCLOUD_HOST", "https://cloud.example.com")
    monkeypatch.setenv("NEXTCLOUD_USERNAME", "admin")
    monkeypatch.setenv("NEXTCLOUD_PASSWORD", "secret")
+    # Ensure multi-user mode is off (may leak from other tests)
+    monkeypatch.delenv("ENABLE_MULTI_USER_BASIC_AUTH", raising=False)
+    _reload_config()


@pytest.mark.unit
@@ -48,6 +52,7 @@ def test_get_stdio_mcp_rejects_non_single_user_mode(monkeypatch):
 def test_get_stdio_mcp_registers_all_apps_by_default(single_user_env):
    """All app tool groups are registered when no filter is specified."""
    mcp = get_stdio_mcp()
+    # NOTE: _tool_manager is a FastMCP internal; may break on SDK upgrades
    tools = mcp._tool_manager.list_tools()
    tool_names = {t.name for t in tools}

@@ -68,6 +73,7 @@ def test_get_stdio_mcp_registers_all_apps_by_default(single_user_env):
 def test_get_stdio_mcp_respects_enabled_apps(single_user_env):
    """Only specified apps have their tools registered."""
    mcp = get_stdio_mcp(enabled_apps=["notes"])
+    # NOTE: _tool_manager is a FastMCP internal; may break on SDK upgrades
    tools = mcp._tool_manager.list_tools()
    tool_names = {t.name for t in tools}

@@ -81,6 +87,7 @@ def test_get_stdio_mcp_respects_enabled_apps(single_user_env):
 def test_get_stdio_mcp_no_semantic_tools(single_user_env):
    """Semantic search tools are never registered in stdio mode."""
    mcp = get_stdio_mcp()
+    # NOTE: _tool_manager is a FastMCP internal; may break on SDK upgrades
    tools = mcp._tool_manager.list_tools()
    tool_names = {t.name for t in tools}

@@ -92,5 +99,6 @@ def test_get_stdio_mcp_no_semantic_tools(single_user_env):
 def test_get_stdio_mcp_registers_capabilities_resource(single_user_env):
    """The nc://capabilities resource is registered."""
    mcp = get_stdio_mcp()
+    # NOTE: _resource_manager._resources is a FastMCP internal; may break on SDK upgrades
    resources = mcp._resource_manager._resources
    assert "nc://capabilities" in resources