diff --git a/README.md b/README.md index adff5e9..d0e22c2 100644 --- a/README.md +++ b/README.md @@ -310,8 +310,10 @@ on Windows): } ``` -Authenticate with your `MASTER_API_KEY`. Each key is limited to 60 requests/min -(configurable via `mcp.rate_limit_rpm`); the 61st in a minute returns `429`. +Authenticate with your `MASTER_API_KEY`. Each key is limited to 60 requests/min; +the 61st in a minute returns `429`. Change it in **Settings → Config → MCP +server** — the new limit applies to the next MCP request, no restart. (Setting +`mcp.rate_limit_rpm` in `config.yaml` works too, but is only read at startup.) **Tools:** `list_workspaces`, `search_documents` (`query`, `collection_ids[]`, `top_k`, `hybrid`, `filters`), `ingest_document` (container-local path), diff --git a/api/dependencies.py b/api/dependencies.py index 79cc049..6951f30 100644 --- a/api/dependencies.py +++ b/api/dependencies.py @@ -7,7 +7,7 @@ from api.adapters.base import EmbeddingAdapter, Reranker from api.adapters.vector_store.pgvector import PgvectorAdapter from api.db import AsyncSessionLocal -from api.models.config import AppConfig, SearchConfig +from api.models.config import AppConfig, MCPConfig, SearchConfig # --------------------------------------------------------------------------- # Adapter singletons — set once in lifespan(), read everywhere via Depends() @@ -36,6 +36,16 @@ def get_search_config() -> SearchConfig: return (_app_config or AppConfig()).search +def get_mcp_config() -> MCPConfig: + """The live MCP config, or defaults before lifespan (mirrors ``get_search_config``). + + Read per request by the MCP rate limiter so a ``rate_limit_rpm`` change applies to + the next request — ``apply_config`` swaps this singleton, and the mounted MCP app + is never rebuilt. + """ + return (_app_config or AppConfig()).mcp + + def set_embedding_adapter(adapter: EmbeddingAdapter) -> None: global _embedding_adapter _embedding_adapter = adapter diff --git a/api/models/config.py b/api/models/config.py index 4f2b4b9..a347020 100644 --- a/api/models/config.py +++ b/api/models/config.py @@ -138,7 +138,12 @@ class ParserConfig(BaseModel): class MCPConfig(BaseModel): enabled: bool = True - rate_limit_rpm: int = 60 + # Per-key requests/min on the mounted /mcp endpoint. The limiter re-reads this on every + # request (see api/services/mcp/rate_limit.py), so a nonsense value bites the moment it is + # saved rather than at the next restart: 0 denies every request, and a negative rate accrues + # token debt that outlives the correction, since the bucket is never rebuilt. Bounded here, + # at the config boundary, so the limiter's hot path needs no clamp — a bad PUT is a clean 422. + rate_limit_rpm: Annotated[int, Field(ge=1)] = 60 max_results: int = 20 diff --git a/api/services/mcp/middleware.py b/api/services/mcp/middleware.py index 6ddbffe..b33da45 100644 --- a/api/services/mcp/middleware.py +++ b/api/services/mcp/middleware.py @@ -91,13 +91,15 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: def build_mcp_middleware( - app: ASGIApp, *, rate_limit_rpm: int, master_key: str | None = None + app: ASGIApp, *, rate_limit_rpm: Callable[[], int], master_key: str | None = None ) -> MCPAuthRateLimitMiddleware: """Wire the MCP middleware from config + the configured master key. Args: app: The MCP ASGI app to protect. - rate_limit_rpm: Requests-per-minute ceiling per API key. + rate_limit_rpm: Returns the requests-per-minute ceiling per API key. Resolved + per request (not captured here), so a config change applies to the next + request — this middleware is built once at mount and never rebuilt. master_key: Override the accepted key (defaults to ``settings.master_api_key``). Returns: diff --git a/api/services/mcp/rate_limit.py b/api/services/mcp/rate_limit.py index 08e4c17..7af71fd 100644 --- a/api/services/mcp/rate_limit.py +++ b/api/services/mcp/rate_limit.py @@ -4,6 +4,13 @@ at ``rpm / 60`` tokens per second. A request consumes one token; when the bucket is empty the request is denied (the transport maps this to HTTP 429). The clock is injectable so the refill behaviour can be tested without sleeping. + +``rpm`` is supplied as a *callable* and resolved on every :meth:`allow` call rather +than captured at construction, so a config change takes effect on the next request +instead of at the next restart: the middleware is built once when the MCP app is +mounted at startup and is never rebuilt (``apply_config`` swaps the live config +singleton, not the mount). A raised limit widens the bucket immediately; a lowered +one is clamped down by the existing refill cap. """ from __future__ import annotations @@ -14,21 +21,18 @@ class TokenBucketRateLimiter: - """A per-key token-bucket limiter sized for "N requests per minute". - - Attributes: - rpm: Bucket capacity and the steady-state requests-per-minute ceiling. - """ + """A per-key token-bucket limiter sized for the *current* "N requests per minute".""" - def __init__(self, rpm: int, *, now: Callable[[], float] = time.monotonic) -> None: + def __init__(self, rpm: Callable[[], int], *, now: Callable[[], float] = time.monotonic) -> None: """Initialise the limiter. Args: - rpm: Maximum requests per minute per key (also the burst capacity). + rpm: Returns the maximum requests per minute per key (also the burst + capacity). Called on every :meth:`allow`, so it can read live config + and a change applies without rebuilding the limiter. now: Monotonic clock returning seconds; injectable for tests. """ - self._capacity = float(rpm) - self._refill_per_sec = rpm / 60.0 + self._rpm = rpm self._now = now self._buckets: dict[str, tuple[float, float]] = {} self._lock = threading.Lock() @@ -43,9 +47,13 @@ def allow(self, key: str) -> bool: ``True`` if a token was available and consumed, ``False`` otherwise. """ with self._lock: + rpm = self._rpm() + capacity = float(rpm) now = self._now() - tokens, last = self._buckets.get(key, (self._capacity, now)) - tokens = min(self._capacity, tokens + (now - last) * self._refill_per_sec) + tokens, last = self._buckets.get(key, (capacity, now)) + # min() re-clamps to the *current* capacity, so a lowered rpm shrinks an + # already-full bucket on the next call instead of letting it drain at the old size. + tokens = min(capacity, tokens + (now - last) * (rpm / 60.0)) allowed = tokens >= 1.0 self._buckets[key] = (tokens - 1.0 if allowed else tokens, now) return allowed diff --git a/api/services/mcp/server.py b/api/services/mcp/server.py index 71d8d99..717777c 100644 --- a/api/services/mcp/server.py +++ b/api/services/mcp/server.py @@ -24,6 +24,7 @@ from api.db import AsyncSessionLocal from api.dependencies import ( get_embedding_adapter, + get_mcp_config, get_reranker, get_search_config, get_vector_store, @@ -137,8 +138,13 @@ def build_mcp_server(*, max_results: int = 20) -> FastMCP: def build_mcp_asgi_app(config: MCPConfig) -> tuple[ASGIApp, FastMCP]: """Build the streamable-HTTP ASGI app for ``/mcp``, guarded by auth + limits. + The rate limit is wired as a *live read* of the ``mcp`` config rather than the + ``config`` snapshot passed here: this app is built once at startup and never + rebuilt, so capturing the value would freeze it until the next restart. Reading + it per request means a config save applies to the very next MCP call. + Args: - config: The resolved ``mcp`` config section (rate limit + result cap). + config: The resolved ``mcp`` config section, for the startup-time result cap. Returns: ``(asgi_app, server)`` — the auth/rate-limit-wrapped streamable-HTTP app, @@ -147,7 +153,9 @@ def build_mcp_asgi_app(config: MCPConfig) -> tuple[ASGIApp, FastMCP]: """ server = build_mcp_server(max_results=config.max_results) http_app = server.streamable_http_app() - guarded = build_mcp_middleware(http_app, rate_limit_rpm=config.rate_limit_rpm) + guarded = build_mcp_middleware( + http_app, rate_limit_rpm=lambda: get_mcp_config().rate_limit_rpm + ) return guarded, server diff --git a/docs/mcp-claude-code.md b/docs/mcp-claude-code.md index ef6f035..f2ba6ea 100644 --- a/docs/mcp-claude-code.md +++ b/docs/mcp-claude-code.md @@ -113,9 +113,12 @@ A typical flow inside a session: - Every request must carry the master key (`Authorization: Bearer …` or `X-API-Key: …`). A missing or wrong key returns **401**. -- Requests are rate-limited per key — **60 requests/min** by default - (`mcp.rate_limit_rpm` in `config.yaml`). The 61st within a minute returns - **429**. +- Requests are rate-limited per key — **60 requests/min** by default. The 61st + within a minute returns **429**. Raise the ceiling in **Settings → Config → + MCP server**: the limiter re-reads the limit on every request, so a save binds + on the next call with no restart. Editing `mcp.rate_limit_rpm` in `config.yaml` + by hand works too, but nothing re-reads the file at runtime — that route only + takes effect when the API restarts. ## Remote / LAN access @@ -139,7 +142,7 @@ own machine, keep `MASTER_API_KEY` strong and set | `✗ Failed to connect`, `404 Not Found` | Wrong path — used `/mcp/` or dropped the `/api` prefix | Use `http://localhost:8000/api/mcp/` (API port, `/api` prefix, trailing slash), or `http:///api/mcp/`. | | Registered as `sse` / old `/api/mcp/sse` URL | Stale SSE registration | Remove and re-add with `--transport http` and the `/api/mcp/` URL (below). | | `401 Missing or invalid API key` | Key not sent or wrong | Check the `Authorization`/`X-API-Key` header matches `MASTER_API_KEY`. | -| `429 Rate limit exceeded` | > 60 req/min on one key | Back off, or raise `mcp.rate_limit_rpm` in `config.yaml`. | +| `429 Rate limit exceeded` | > 60 req/min on one key | Back off, or raise the limit in Settings → Config → MCP server (applies immediately). | | Connection refused | Stack not up / wrong port | `docker compose ps`; confirm the API port and `curl /healthz`. | | `search_documents` errors / 500 | Embedding provider down | Start Ollama (`ollama serve`) and `ollama pull embeddinggemma`. | diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 1cc22a3..f04d76d 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -2887,6 +2887,7 @@ components: default: true rate_limit_rpm: type: integer + minimum: 1.0 title: Rate Limit Rpm default: 60 max_results: diff --git a/tests/integration/test_mcp.py b/tests/integration/test_mcp.py index ac6c783..953bafb 100644 --- a/tests/integration/test_mcp.py +++ b/tests/integration/test_mcp.py @@ -257,7 +257,9 @@ async def _ok_app(scope, receive, send): def _guarded(rpm: int) -> MCPAuthRateLimitMiddleware: return MCPAuthRateLimitMiddleware( - _ok_app, authenticate=lambda k: k == "secret", rate_limiter=TokenBucketRateLimiter(rpm) + _ok_app, + authenticate=lambda k: k == "secret", + rate_limiter=TokenBucketRateLimiter(lambda: rpm), ) diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 2a25301..bcc4614 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -2,6 +2,9 @@ import warnings +import pytest +from pydantic import ValidationError + from api.constants import POSTGRES_PORT from api.models.config import AppConfig @@ -19,6 +22,20 @@ def test_defaults_are_populated(): assert cfg.mcp.rate_limit_rpm == 60 +@pytest.mark.parametrize("rpm", [0, -60]) +def test_mcp_rate_limit_rejects_values_below_one(rpm): + """The MCP limiter re-reads rate_limit_rpm per request, so a 0/negative value would brick the + endpoint the instant it is saved — 0 denies everything, and a negative rate accrues token debt + that outlives the correction (the bucket is never rebuilt). Reject it at the boundary: a bad + PUT /config is a 422 instead of a live lockout.""" + with pytest.raises(ValidationError): + AppConfig.model_validate({"mcp": {"rate_limit_rpm": rpm}}) + + +def test_mcp_rate_limit_accepts_one(): + assert AppConfig.model_validate({"mcp": {"rate_limit_rpm": 1}}).mcp.rate_limit_rpm == 1 + + def test_nested_override_applies(): cfg = AppConfig.model_validate( { diff --git a/tests/unit/test_mcp_rate_limit.py b/tests/unit/test_mcp_rate_limit.py index 176fdd0..78e1bd8 100644 --- a/tests/unit/test_mcp_rate_limit.py +++ b/tests/unit/test_mcp_rate_limit.py @@ -2,7 +2,8 @@ Drives :class:`TokenBucketRateLimiter` with an injected fake clock so the "60 requests per minute, 61st denied" behaviour is asserted deterministically -without sleeping. +without sleeping. The rpm provider is likewise injected, which also covers the +limit changing live (a config save) without the limiter being rebuilt. """ from __future__ import annotations @@ -25,7 +26,7 @@ def advance(self, seconds: float) -> None: def test_allows_up_to_capacity_then_denies() -> None: clock = _FakeClock() - limiter = TokenBucketRateLimiter(rpm=60, now=clock) + limiter = TokenBucketRateLimiter(rpm=lambda: 60, now=clock) # All 60 tokens are available immediately (no time has elapsed). assert all(limiter.allow("key-a") for _ in range(60)) @@ -35,7 +36,7 @@ def test_allows_up_to_capacity_then_denies() -> None: def test_refills_one_token_per_second() -> None: clock = _FakeClock() - limiter = TokenBucketRateLimiter(rpm=60, now=clock) + limiter = TokenBucketRateLimiter(rpm=lambda: 60, now=clock) for _ in range(60): limiter.allow("key-a") assert limiter.allow("key-a") is False @@ -47,7 +48,7 @@ def test_refills_one_token_per_second() -> None: def test_refill_is_capped_at_capacity() -> None: clock = _FakeClock() - limiter = TokenBucketRateLimiter(rpm=60, now=clock) + limiter = TokenBucketRateLimiter(rpm=lambda: 60, now=clock) limiter.allow("key-a") # consume one clock.advance(3600.0) # an hour of idle must not exceed capacity @@ -56,7 +57,7 @@ def test_refill_is_capped_at_capacity() -> None: def test_buckets_are_isolated_per_key() -> None: clock = _FakeClock() - limiter = TokenBucketRateLimiter(rpm=5, now=clock) + limiter = TokenBucketRateLimiter(rpm=lambda: 5, now=clock) assert all(limiter.allow("key-a") for _ in range(5)) assert limiter.allow("key-a") is False # A different key has its own full bucket. @@ -66,7 +67,37 @@ def test_buckets_are_isolated_per_key() -> None: def test_rpm_is_configurable() -> None: clock = _FakeClock() - limiter = TokenBucketRateLimiter(rpm=2, now=clock) + limiter = TokenBucketRateLimiter(rpm=lambda: 2, now=clock) assert limiter.allow("k") is True assert limiter.allow("k") is True assert limiter.allow("k") is False + + +def test_raised_rpm_applies_without_rebuilding_the_limiter() -> None: + """A config save must take effect on the next request, not the next restart. + + Regression guard: rpm used to be captured in ``__init__``, so the limiter built when + the MCP app was mounted at startup kept the boot-time limit forever — ``apply_config`` + swaps the live config singleton but never rebuilds the mount. + """ + clock = _FakeClock() + rpm = 2 + limiter = TokenBucketRateLimiter(rpm=lambda: rpm, now=clock) + assert all(limiter.allow("k") for _ in range(2)) + assert limiter.allow("k") is False # bucket empty at the old limit + + rpm = 60 # the config PUT lands — same limiter instance, no restart + clock.advance(1.0) # 60 rpm => 1 token/sec, the NEW refill rate + assert limiter.allow("k") is True + + +def test_lowered_rpm_shrinks_an_already_full_bucket() -> None: + """Dropping the limit must bite immediately — a bucket filled at the old, higher + capacity is re-clamped on the next call instead of serving one last oversized burst.""" + clock = _FakeClock() + rpm = 60 + limiter = TokenBucketRateLimiter(rpm=lambda: rpm, now=clock) + limiter.allow("k") # bucket sitting at 59 tokens under the old capacity + + rpm = 5 + assert sum(limiter.allow("k") for _ in range(100)) == 5 # clamped to the new capacity diff --git a/ui/src/api/types.ts b/ui/src/api/types.ts index fda579c..aafc2de 100644 --- a/ui/src/api/types.ts +++ b/ui/src/api/types.ts @@ -447,6 +447,18 @@ export interface StorageConfig { temp_retention_hours: number // 0 = temporary uploads never expire (feature off) } +/** + * MCP transport config. `rate_limit_rpm` caps requests per minute per API key on the + * mounted `/api/mcp` endpoint; the limiter reads it from live config per request, so a + * save applies to the next MCP call. `enabled` needs a restart (it drives the mount) and + * `max_results` is read at startup; both round-trip untouched from the UI. + */ +export interface MCPConfig { + enabled: boolean + rate_limit_rpm: number + max_results: number +} + /** `GET /config/accelerator` — GPU suitability for the docling PDF backend. */ export interface Accelerator { device: string // "cuda" | "cpu" @@ -465,6 +477,7 @@ export interface AppConfig { search: SearchConfig parsers: ParserConfig storage: StorageConfig + mcp: MCPConfig [section: string]: unknown } diff --git a/ui/src/components/settings/ConfigPanel.tsx b/ui/src/components/settings/ConfigPanel.tsx index f743fe8..d2cbdcc 100644 --- a/ui/src/components/settings/ConfigPanel.tsx +++ b/ui/src/components/settings/ConfigPanel.tsx @@ -10,6 +10,7 @@ import { import type { AppConfig, EmbeddingConfig, + MCPConfig, RerankerConfig, SearchConfig, } from '../../api/types' @@ -35,6 +36,7 @@ export function ConfigPanel() { + ) } @@ -266,7 +268,7 @@ function EmbeddingForm({ config }: { config: AppConfig }) {

The embedding model turns documents into vectors. Changing the provider, - model, or output dimensions changes the vector size — existing collections + model, or output dimensions changes the vector size — existing collections must be re-indexed or search will break. Changing only an API key is safe.

@@ -640,6 +642,75 @@ function SearchForm({ config }: { config: AppConfig }) { ) } +/** + * MCP server config. The one editable knob is the per-key request rate limit on the mounted + * `/api/mcp` endpoint (the 61st request in a minute → 429 by default). Applies live, like the + * sections above: the limiter reads `mcp.rate_limit_rpm` from the live config on every request, + * so a save binds from the next MCP call. `enabled` and `max_results` round-trip untouched. + */ +function MCPForm({ config }: { config: AppConfig }) { + const toast = useToast() + const update = useUpdateConfig() + const mcp = config.mcp + + const [rpm, setRpm] = useState(String(mcp.rate_limit_rpm)) + + const save = () => { + // The backend takes a strict int >= 1 and 422s anything else. Blank (or unparseable) reverts + // to the saved value; everything else is rounded and floored at 1. Test the entry for blank + // up front rather than leaning on `Number(x) || fallback` — `Number('0')` is falsy, so that + // idiom would divert 0 to the fallback and never floor it. + const entered = rpm.trim() + const parsed = Math.round(Number(entered)) + const rateLimitRpm = + entered === '' || !Number.isFinite(parsed) ? mcp.rate_limit_rpm : Math.max(1, parsed) + const next: MCPConfig = { ...mcp, rate_limit_rpm: rateLimitRpm } + setRpm(String(rateLimitRpm)) // show what was actually saved, not the raw entry + update.mutate( + { ...config, mcp: next }, + { + onSuccess: () => toast.success('MCP config saved. Services are reloading.'), + onError: (e) => toast.error(e.message), + }, + ) + } + + return ( + +
+ +

+ The MCP endpoint (/api/mcp) throttles each API key to this many requests per + minute; the next request in the same minute returns 429. Each key gets its own + budget, and the new limit applies to the next MCP request +

+
+ +
+ + setRpm(e.target.value)} + /> + +
+ +
+ +
+ + ) +} + /** * Model picker for Ollama: a Select populated from the server's installed models. * No free-text entry — if a model isn't installed it can't be chosen. Auto-selects