From fc8a4e4dfa03eca7f52bf2bb224f15f1ce53bf34 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Mon, 8 Jun 2026 14:59:03 +0200 Subject: [PATCH 1/2] fix(documents): guard Unix-only resource import for Windows (#877) `document_processors/_isolation.py` did an unconditional module-level `import resource`, a POSIX-only stdlib module absent on Windows. It was pulled into the API startup path via `server/webdav.py -> utils/document_parser -> document_processors`, so the MCP server failed to start on Windows since 0.101.2 with `ModuleNotFoundError: No module named 'resource'`. - Guard the import behind `sys.platform`; bind `resource = None` on win32. `_apply_mem_limit()` degrades to a logged no-op when the module is unavailable (the RLIMIT_AS cap is a Linux-pod safety measure, not a correctness requirement). - Make the document-parser import in `server/webdav.py` lazy so server startup never loads the ingest document stack (document_processors -> pymupdf -> _isolation) at all -- it is only needed when a file is actually read and parsed. This both fixes #877 and decouples the API layer from ingest-only deps. - Add unit regressions for the no-op path and the win32 import guard. - Add a cross-platform `package-smoke` CI job (ubuntu + windows) that installs the package isolated and runs the CLI, exercising the cli -> server -> webdav import chain that crashed in #877. Fixes #877 Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/test.yml | 21 +++++++++++ .../document_processors/_isolation.py | 19 +++++++++- nextcloud_mcp_server/server/webdav.py | 14 +++++--- tests/unit/test_pdf_parse_isolation.py | 36 +++++++++++++++++++ 4 files changed, 85 insertions(+), 5 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5fa68f1d..12dd71b3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -29,6 +29,27 @@ jobs: - name: Run unit tests run: uv run pytest -v -m unit -o "addopts=-p no:asyncio" + # Cross-platform install smoke test. Installs the package (and its full + # dependency closure) into an isolated environment and runs the CLI + # entrypoint, which exercises the cli -> server -> webdav import chain. This is + # the regression guard for #877, where a Unix-only ``import resource`` in that + # chain crashed Windows startup. Runs on Windows in addition to Linux so any + # platform-specific import regression fails here. + package-smoke: + needs: [linting] + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + runs-on: ${{ matrix.os }} + name: package-smoke (${{ matrix.os }}) + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + - name: Install the latest version of uv + uses: astral-sh/setup-uv@37802adc94f370d6bfd71619e3f0bf239e1f3b78 # v7.6.0 + - name: Smoke test CLI (isolated install) + run: uv run --isolated --no-project --with . nextcloud-mcp-server --help + integration-test: runs-on: ubuntu-latest needs: [linting] diff --git a/nextcloud_mcp_server/document_processors/_isolation.py b/nextcloud_mcp_server/document_processors/_isolation.py index f030bbe4..dd6ff20e 100644 --- a/nextcloud_mcp_server/document_processors/_isolation.py +++ b/nextcloud_mcp_server/document_processors/_isolation.py @@ -13,7 +13,7 @@ in its process pool. """ import logging -import resource +import sys from pathlib import Path from typing import Any @@ -21,6 +21,15 @@ import anyio import anyio.to_process from anyio import BrokenWorkerProcess +# ``resource`` is a Unix-only stdlib module -- it does not exist on Windows, and +# importing it unconditionally crashed Windows startup (#877). The RLIMIT_AS cap +# it provides is a Linux-pod safety measure, not a correctness requirement, so on +# platforms without it we fall back to a no-op (``resource is None``). +if sys.platform == "win32": # pragma: no cover - exercised only on Windows + resource = None +else: + import resource + logger = logging.getLogger(__name__) # Guard so the address-space limit is applied once per (reused) worker process. @@ -44,10 +53,18 @@ def _apply_mem_limit(mem_limit_mb: int) -> None: Applied once per worker process. The hard limit is left untouched (we only lower the soft limit), and we never set a soft limit above the hard limit. + + On a platform without the Unix-only ``resource`` module (e.g. Windows, see + #877) the cap is skipped -- the worker still runs, just without the + address-space limit. """ global _MEM_LIMIT_APPLIED if _MEM_LIMIT_APPLIED or mem_limit_mb <= 0: return + if resource is None: + logger.debug("resource module unavailable; skipping RLIMIT_AS cap") + _MEM_LIMIT_APPLIED = True + return target = mem_limit_mb * 1024 * 1024 soft, hard = resource.getrlimit(resource.RLIMIT_AS) soft_target = target if hard == resource.RLIM_INFINITY else min(target, hard) diff --git a/nextcloud_mcp_server/server/webdav.py b/nextcloud_mcp_server/server/webdav.py index 39ce775b..ebb04b7e 100644 --- a/nextcloud_mcp_server/server/webdav.py +++ b/nextcloud_mcp_server/server/webdav.py @@ -13,10 +13,6 @@ from nextcloud_mcp_server.server.tag_exclusion import ( get_excluded_file_paths, is_path_excluded, ) -from nextcloud_mcp_server.utils.document_parser import ( - is_parseable_document, - parse_document, -) logger = logging.getLogger(__name__) @@ -115,6 +111,16 @@ def configure_webdav_tools(mcp: FastMCP): content, content_type = await client.webdav.read_file(path) + # Imported lazily so server startup never loads the document-parsing + # stack (document_processors -> pymupdf -> _isolation). That stack is an + # ingest-layer concern and, before this, broke Windows startup via a + # Unix-only ``import resource`` (#877). It is only needed when a file is + # actually read and parsed. + from nextcloud_mcp_server.utils.document_parser import ( # noqa: PLC0415 + is_parseable_document, + parse_document, + ) + # Check if this is a parseable document (PDF, DOCX, etc.) # is_parseable_document() checks if document processing is enabled if is_parseable_document(content_type): diff --git a/tests/unit/test_pdf_parse_isolation.py b/tests/unit/test_pdf_parse_isolation.py index ebafe8f9..06a7bcbb 100644 --- a/tests/unit/test_pdf_parse_isolation.py +++ b/tests/unit/test_pdf_parse_isolation.py @@ -14,6 +14,7 @@ or depend on the sample files). """ import resource +import sys import anyio import anyio.to_process @@ -161,6 +162,41 @@ def test_apply_mem_limit_is_applied_once(monkeypatch): assert len(calls) == 1 # second call is a no-op +# --- Windows / no-``resource`` platform compatibility (#877) ----------------- + + +def test_apply_mem_limit_noop_when_resource_unavailable(monkeypatch): + """On a platform without ``resource`` (e.g. Windows) the cap is skipped. + + Regression for #877: ``resource`` is Unix-only, so ``_apply_mem_limit`` must + degrade to a no-op (rather than crash) when the module is unavailable. + """ + monkeypatch.setattr(_isolation, "_MEM_LIMIT_APPLIED", False) + monkeypatch.setattr(_isolation, "resource", None) + _isolation._apply_mem_limit(1536) # must not raise + assert _isolation._MEM_LIMIT_APPLIED is True + + +def test_isolation_imports_on_windows_without_resource(monkeypatch): + """Importing ``_isolation`` on Windows must not crash on ``import resource``. + + Regression for #877: a module-scope ``import resource`` raised + ``ModuleNotFoundError`` on Windows and took down server startup. With + ``sys.platform == 'win32'`` the module must import cleanly and bind + ``resource`` to ``None``. + """ + import importlib + + monkeypatch.setattr(sys, "platform", "win32") + monkeypatch.delitem( + sys.modules, + "nextcloud_mcp_server.document_processors._isolation", + raising=False, + ) + mod = importlib.import_module("nextcloud_mcp_server.document_processors._isolation") + assert mod.resource is None + + # --- PyMuPDF processor wiring ------------------------------------------------ From 62274069dede8a36073efa9b74432939c83c4fc0 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Mon, 8 Jun 2026 15:11:18 +0200 Subject: [PATCH 2/2] refactor(documents): fully decouple document stack from server startup; Windows-safe tests Addresses round-1 review on #878: - Move the eager `document_processors` imports out of the API startup graph: `app.py` (get_registry now imported inside initialize_document_processors, after the disabled early-return) and `vector/processor.py` (get_registry now imported at its single use site). Importing `app` + `cli` no longer loads `document_processors` / `_isolation` at all -- the #877 stack is fully out of startup (pymupdf still loads via search/pdf_highlighter, a Windows-compatible and separately-tracked concern). - Make `tests/unit/test_pdf_parse_isolation.py` importable on Windows: guard the top-level `import resource` with try/except and skip the three rlimit computation tests via a `requires_resource` marker when the module is absent. The Windows no-op / import-guard tests don't use the real module and still run. - Fix the `# pragma: no cover` comment on the win32 branch to be accurate. - Add `enable-cache: true` to the package-smoke setup-uv step. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/test.yml | 2 ++ nextcloud_mcp_server/app.py | 6 +++++- .../document_processors/_isolation.py | 2 +- nextcloud_mcp_server/vector/processor.py | 7 ++++++- tests/unit/test_pdf_parse_isolation.py | 16 +++++++++++++++- 5 files changed, 29 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 12dd71b3..3bcc0781 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -47,6 +47,8 @@ jobs: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Install the latest version of uv uses: astral-sh/setup-uv@37802adc94f370d6bfd71619e3f0bf239e1f3b78 # v7.6.0 + with: + enable-cache: true - name: Smoke test CLI (isolated install) run: uv run --isolated --no-project --with . nextcloud-mcp-server --help diff --git a/nextcloud_mcp_server/app.py b/nextcloud_mcp_server/app.py index feace0ee..838480f2 100644 --- a/nextcloud_mcp_server/app.py +++ b/nextcloud_mcp_server/app.py @@ -109,7 +109,6 @@ from nextcloud_mcp_server.config_validators import ( validate_configuration, ) from nextcloud_mcp_server.context import get_client as get_nextcloud_client -from nextcloud_mcp_server.document_processors import get_registry from nextcloud_mcp_server.http import nextcloud_httpx_client from nextcloud_mcp_server.observability import ( ObservabilityMiddleware, @@ -159,6 +158,11 @@ def initialize_document_processors(): logger.info("Document processing disabled") return + # Imported lazily so the API startup path never loads the ingest document + # stack (document_processors -> pymupdf -> _isolation) unless document + # processing is actually enabled -- see #877 / the API-vs-ingest split. + from nextcloud_mcp_server.document_processors import get_registry # noqa: PLC0415 + registry = get_registry() registered_count = 0 diff --git a/nextcloud_mcp_server/document_processors/_isolation.py b/nextcloud_mcp_server/document_processors/_isolation.py index dd6ff20e..6e5aafb7 100644 --- a/nextcloud_mcp_server/document_processors/_isolation.py +++ b/nextcloud_mcp_server/document_processors/_isolation.py @@ -25,7 +25,7 @@ from anyio import BrokenWorkerProcess # importing it unconditionally crashed Windows startup (#877). The RLIMIT_AS cap # it provides is a Linux-pod safety measure, not a correctness requirement, so on # platforms without it we fall back to a no-op (``resource is None``). -if sys.platform == "win32": # pragma: no cover - exercised only on Windows +if sys.platform == "win32": # pragma: no cover - win32-only path resource = None else: import resource diff --git a/nextcloud_mcp_server/vector/processor.py b/nextcloud_mcp_server/vector/processor.py index b9bd2472..bde4c375 100644 --- a/nextcloud_mcp_server/vector/processor.py +++ b/nextcloud_mcp_server/vector/processor.py @@ -16,7 +16,6 @@ from qdrant_client.models import PointStruct from nextcloud_mcp_server.acl_hash import compute_acl_hash from nextcloud_mcp_server.client import NextcloudClient from nextcloud_mcp_server.config import get_settings -from nextcloud_mcp_server.document_processors import get_registry from nextcloud_mcp_server.embedding import get_bm25_service, get_embedding_service from nextcloud_mcp_server.models.deck import DeckCard from nextcloud_mcp_server.observability.metrics import ( @@ -628,6 +627,12 @@ async def _index_document( ): # The registry runs the tiered PDF pipeline (tier-0 classify -> # tier-1 fast -> OCR escalation) and records classification metrics. + # Imported lazily so module import doesn't pull in the document stack + # (document_processors -> _isolation, Unix-only ``resource``; see #877). + from nextcloud_mcp_server.document_processors import ( # noqa: PLC0415 + get_registry, + ) + registry = get_registry() try: diff --git a/tests/unit/test_pdf_parse_isolation.py b/tests/unit/test_pdf_parse_isolation.py index 06a7bcbb..899bc607 100644 --- a/tests/unit/test_pdf_parse_isolation.py +++ b/tests/unit/test_pdf_parse_isolation.py @@ -13,7 +13,6 @@ check on the sample PDFs, not here (unit tests must not spawn the heavy worker or depend on the sample files). """ -import resource import sys import anyio @@ -28,8 +27,20 @@ from nextcloud_mcp_server.document_processors._isolation import ( run_isolated_pdf_parse, ) +# ``resource`` is a Unix-only stdlib module (absent on Windows, #877). Guard the +# import so this test module stays importable on Windows; the rlimit-computation +# tests below are skipped there via the ``requires_resource`` marker. +try: + import resource +except ImportError: # pragma: no cover - only reached on Windows + resource = None # type: ignore[assignment] + pytestmark = pytest.mark.unit +requires_resource = pytest.mark.skipif( + resource is None, reason="resource module is Unix-only (absent on Windows)" +) + def _tiny_pdf() -> bytes: doc = pymupdf.open() @@ -112,6 +123,7 @@ async def test_timeout_kills_and_classifies_as_timeout(monkeypatch): # --- _apply_mem_limit computation (mocked; never applied to the test proc) --- +@requires_resource def test_apply_mem_limit_caps_soft_below_finite_hard(monkeypatch): captured = {} monkeypatch.setattr(_isolation, "_MEM_LIMIT_APPLIED", False) @@ -130,6 +142,7 @@ def test_apply_mem_limit_caps_soft_below_finite_hard(monkeypatch): assert hard == 4 * 1024**3 +@requires_resource def test_apply_mem_limit_uses_target_when_hard_unlimited(monkeypatch): captured = {} monkeypatch.setattr(_isolation, "_MEM_LIMIT_APPLIED", False) @@ -148,6 +161,7 @@ def test_apply_mem_limit_uses_target_when_hard_unlimited(monkeypatch): assert hard == resource.RLIM_INFINITY +@requires_resource def test_apply_mem_limit_is_applied_once(monkeypatch): calls = [] monkeypatch.setattr(_isolation, "_MEM_LIMIT_APPLIED", False)