Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions nextcloud_mcp_server/models/tables.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ class Table(BaseModel):
id: int = Field(description="Table ID")
title: str = Field(description="Table title")
emoji: Optional[str] = Field(None, description="Table emoji")
ownership: str = Field(description="Table ownership")
# Same failure mode as owner_display_name below: a required field here fails
# the *whole* nc_tables_list_tables call (server/tables.py splats each dict
# straight into Table), so one absent key costs every table. Nothing in this
# codebase reads `ownership`, so Optional costs nothing.
ownership: Optional[str] = Field(None, description="Table ownership")
# Tables app v2.0.1 stopped emitting owner_display_name at the top level
# (still present inside views via get_schema). Optional avoids a 100% failure
# rate on list_tables — see #728.
Expand Down Expand Up @@ -99,8 +103,13 @@ class TableSchema(BaseModel):
"""Model for complete table schema including columns and views."""

table: Table = Field(description="Table information")
columns: List[TableColumn] = Field(description="Table columns")
views: List[TableView] = Field(description="Table views")
# default_factory for the same reason: a schema response that omits either
# list (an empty table has no views) should degrade to an empty list rather
# than failing the call.
columns: List[TableColumn] = Field(
default_factory=list, description="Table columns"
)
views: List[TableView] = Field(default_factory=list, description="Table views")


class ListTablesResponse(BaseResponse):
Expand Down
21 changes: 19 additions & 2 deletions nextcloud_mcp_server/providers/anthropic.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import logging

import httpx
from anthropic import AsyncAnthropic
from anthropic.types import TextBlock

Expand All @@ -18,17 +19,33 @@ class AnthropicProvider(Provider):
Note: Anthropic doesn't provide embedding models, only text generation.
"""

# 120s read / 5s connect is the house convention (ollama.py, openai.py). The
# Anthropic SDK otherwise defaults to 600s, long enough that a wedged request
# looks like a hang rather than a failure.
DEFAULT_TIMEOUT_SECONDS = 120.0
DEFAULT_CONNECT_TIMEOUT_SECONDS = 5.0

def __init__(
self, api_key: str, generation_model: str = "claude-3-5-sonnet-20241022"
self,
api_key: str,
generation_model: str = "claude-3-5-sonnet-20241022",
timeout: httpx.Timeout | None = None,
):
"""
Initialize Anthropic provider.

Args:
api_key: Anthropic API key
generation_model: Model name (e.g., "claude-3-5-sonnet-20241022")
timeout: Optional httpx timeout. Defaults to 120s read / 5s connect,
matching the other providers.
"""
self.client = AsyncAnthropic(api_key=api_key)
if timeout is None:
timeout = httpx.Timeout(
timeout=self.DEFAULT_TIMEOUT_SECONDS,
connect=self.DEFAULT_CONNECT_TIMEOUT_SECONDS,
)
self.client = AsyncAnthropic(api_key=api_key, timeout=timeout)
self.model = generation_model

logger.info("Initialized Anthropic provider (model=%s)", self.model)
Expand Down
19 changes: 17 additions & 2 deletions nextcloud_mcp_server/providers/bedrock.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

try:
import boto3
from botocore.config import Config as BotoConfig
from botocore.exceptions import BotoCoreError, ClientError

BOTO3_AVAILABLE = True
Expand All @@ -31,6 +32,10 @@ class BedrockProvider(Provider):
- IAM role (when running on AWS)
"""

# Matches ollama.py / openai.py / anthropic.py.
DEFAULT_TIMEOUT_SECONDS = 120
DEFAULT_CONNECT_TIMEOUT_SECONDS = 5

def __init__(
self,
region_name: str | None = None,
Expand Down Expand Up @@ -63,8 +68,18 @@ def __init__(
self.generation_model = generation_model
self._dimension: int | None = None # Detected dynamically

# Initialize bedrock-runtime client
client_kwargs: dict[str, Any] = {}
# Initialize bedrock-runtime client.
#
# botocore's defaults are 60s connect and 60s read with 3 retries, so a
# wedged endpoint can hold a request for minutes. Pin the same 120s read
# / 5s connect the other providers use; retries stay at botocore's
# default since Bedrock throttling is expected and handled upstream.
client_kwargs: dict[str, Any] = {
"config": BotoConfig(
connect_timeout=self.DEFAULT_CONNECT_TIMEOUT_SECONDS,
read_timeout=self.DEFAULT_TIMEOUT_SECONDS,
)
}
if region_name:
client_kwargs["region_name"] = region_name
if aws_access_key_id:
Expand Down
55 changes: 55 additions & 0 deletions tests/unit/providers/test_provider_timeouts.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
"""Every provider must pin an explicit timeout.

The house convention is 120s read / 5s connect (ollama.py, openai.py). Without
it the underlying SDKs fall back to their own defaults — 600s for the Anthropic
SDK, 60s read with 3 retries for botocore — long enough that a wedged endpoint
looks like a hang rather than a failure.
"""

from __future__ import annotations

import pytest

pytestmark = pytest.mark.unit


def test_anthropic_provider_pins_a_timeout(mocker):
from nextcloud_mcp_server.providers.anthropic import AnthropicProvider

ctor = mocker.patch("nextcloud_mcp_server.providers.anthropic.AsyncAnthropic")

AnthropicProvider(api_key="k")

timeout = ctor.call_args.kwargs["timeout"]
assert timeout.read == AnthropicProvider.DEFAULT_TIMEOUT_SECONDS
assert timeout.connect == AnthropicProvider.DEFAULT_CONNECT_TIMEOUT_SECONDS


def test_anthropic_provider_accepts_an_explicit_timeout(mocker):
import httpx

from nextcloud_mcp_server.providers.anthropic import AnthropicProvider

ctor = mocker.patch("nextcloud_mcp_server.providers.anthropic.AsyncAnthropic")
supplied = httpx.Timeout(timeout=7, connect=1)

AnthropicProvider(api_key="k", timeout=supplied)

assert ctor.call_args.kwargs["timeout"] is supplied


def test_bedrock_provider_pins_botocore_timeouts(mocker):
bedrock = pytest.importorskip("nextcloud_mcp_server.providers.bedrock")
if not bedrock.BOTO3_AVAILABLE:
pytest.skip("boto3 not installed")

client = mocker.patch.object(bedrock.boto3, "client")

bedrock.BedrockProvider(region_name="us-east-1")

config = client.call_args.kwargs["config"]
assert (
config.connect_timeout
== bedrock.BedrockProvider.DEFAULT_CONNECT_TIMEOUT_SECONDS
)
assert config.read_timeout == bedrock.BedrockProvider.DEFAULT_TIMEOUT_SECONDS
25 changes: 24 additions & 1 deletion tests/unit/test_response_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
SamplingSearchResponse,
SemanticSearchResult,
)
from nextcloud_mcp_server.models.tables import Table
from nextcloud_mcp_server.models.tables import Table, TableSchema
from nextcloud_mcp_server.server.calendar import _event_dict_to_summary
from nextcloud_mcp_server.server.contacts import _raw_contact_to_model

Expand Down Expand Up @@ -1042,3 +1042,26 @@ def test_contact_mapping_truncates_overlong_address_components():

assert len(contact.addresses[0].components) == 7
assert contact.addresses[0].components == ["0", "1", "2", "3", "4", "5", "6"]


@pytest.mark.unit
def test_table_tolerates_missing_ownership():
"""A required `ownership` failed the *entire* nc_tables_list_tables call.

server/tables.py splats each raw dict straight into Table, so one table
missing the key costs every table in the response — the same failure mode as
owner_display_name in #728. Nothing in the codebase reads `ownership`.
"""
table = Table(id=1, title="Budget")

assert table.ownership is None
assert table.owner_display_name is None


@pytest.mark.unit
def test_table_schema_tolerates_missing_columns_and_views():
"""An empty table has no views; a schema response may omit either list."""
schema = TableSchema(table=Table(id=1, title="Budget"))

assert schema.columns == []
assert schema.views == []