Merge pull request #878 from cbcoutinho/fix/windows-resource-import-877

fix(documents): guard Unix-only resource import for Windows (#877)
This commit is contained in:
Chris Coutinho
2026-06-08 16:22:42 +02:00
committed by GitHub
6 changed files with 113 additions and 8 deletions
+23
View File
@@ -29,6 +29,29 @@ jobs:
- name: Run unit tests - name: Run unit tests
run: uv run pytest -v -m unit -o "addopts=-p no:asyncio" 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
with:
enable-cache: true
- name: Smoke test CLI (isolated install)
run: uv run --isolated --no-project --with . nextcloud-mcp-server --help
integration-test: integration-test:
runs-on: ubuntu-latest runs-on: ubuntu-latest
needs: [linting] needs: [linting]
+5 -1
View File
@@ -109,7 +109,6 @@ from nextcloud_mcp_server.config_validators import (
validate_configuration, validate_configuration,
) )
from nextcloud_mcp_server.context import get_client as get_nextcloud_client 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.http import nextcloud_httpx_client
from nextcloud_mcp_server.observability import ( from nextcloud_mcp_server.observability import (
ObservabilityMiddleware, ObservabilityMiddleware,
@@ -159,6 +158,11 @@ def initialize_document_processors():
logger.info("Document processing disabled") logger.info("Document processing disabled")
return 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() registry = get_registry()
registered_count = 0 registered_count = 0
@@ -13,7 +13,7 @@ in its process pool.
""" """
import logging import logging
import resource import sys
from pathlib import Path from pathlib import Path
from typing import Any from typing import Any
@@ -21,6 +21,15 @@ import anyio
import anyio.to_process import anyio.to_process
from anyio import BrokenWorkerProcess 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 - win32-only path
resource = None
else:
import resource
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
# Guard so the address-space limit is applied once per (reused) worker process. # 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 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. 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 global _MEM_LIMIT_APPLIED
if _MEM_LIMIT_APPLIED or mem_limit_mb <= 0: if _MEM_LIMIT_APPLIED or mem_limit_mb <= 0:
return 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 target = mem_limit_mb * 1024 * 1024
soft, hard = resource.getrlimit(resource.RLIMIT_AS) soft, hard = resource.getrlimit(resource.RLIMIT_AS)
soft_target = target if hard == resource.RLIM_INFINITY else min(target, hard) soft_target = target if hard == resource.RLIM_INFINITY else min(target, hard)
+10 -4
View File
@@ -13,10 +13,6 @@ from nextcloud_mcp_server.server.tag_exclusion import (
get_excluded_file_paths, get_excluded_file_paths,
is_path_excluded, is_path_excluded,
) )
from nextcloud_mcp_server.utils.document_parser import (
is_parseable_document,
parse_document,
)
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
@@ -115,6 +111,16 @@ def configure_webdav_tools(mcp: FastMCP):
content, content_type = await client.webdav.read_file(path) 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.) # Check if this is a parseable document (PDF, DOCX, etc.)
# is_parseable_document() checks if document processing is enabled # is_parseable_document() checks if document processing is enabled
if is_parseable_document(content_type): if is_parseable_document(content_type):
+6 -1
View File
@@ -16,7 +16,6 @@ from qdrant_client.models import PointStruct
from nextcloud_mcp_server.acl_hash import compute_acl_hash from nextcloud_mcp_server.acl_hash import compute_acl_hash
from nextcloud_mcp_server.client import NextcloudClient from nextcloud_mcp_server.client import NextcloudClient
from nextcloud_mcp_server.config import get_settings 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.embedding import get_bm25_service, get_embedding_service
from nextcloud_mcp_server.models.deck import DeckCard from nextcloud_mcp_server.models.deck import DeckCard
from nextcloud_mcp_server.observability.metrics import ( from nextcloud_mcp_server.observability.metrics import (
@@ -707,6 +706,12 @@ async def _index_document(
): ):
# The registry runs the tiered PDF pipeline (tier-0 classify -> # The registry runs the tiered PDF pipeline (tier-0 classify ->
# tier-1 fast -> OCR escalation) and records classification metrics. # 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() registry = get_registry()
try: try:
+51 -1
View File
@@ -13,7 +13,7 @@ check on the sample PDFs, not here (unit tests must not spawn the heavy worker
or depend on the sample files). or depend on the sample files).
""" """
import resource import sys
import anyio import anyio
import anyio.to_process import anyio.to_process
@@ -27,8 +27,20 @@ from nextcloud_mcp_server.document_processors._isolation import (
run_isolated_pdf_parse, 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 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: def _tiny_pdf() -> bytes:
doc = pymupdf.open() doc = pymupdf.open()
@@ -111,6 +123,7 @@ async def test_timeout_kills_and_classifies_as_timeout(monkeypatch):
# --- _apply_mem_limit computation (mocked; never applied to the test proc) --- # --- _apply_mem_limit computation (mocked; never applied to the test proc) ---
@requires_resource
def test_apply_mem_limit_caps_soft_below_finite_hard(monkeypatch): def test_apply_mem_limit_caps_soft_below_finite_hard(monkeypatch):
captured = {} captured = {}
monkeypatch.setattr(_isolation, "_MEM_LIMIT_APPLIED", False) monkeypatch.setattr(_isolation, "_MEM_LIMIT_APPLIED", False)
@@ -129,6 +142,7 @@ def test_apply_mem_limit_caps_soft_below_finite_hard(monkeypatch):
assert hard == 4 * 1024**3 assert hard == 4 * 1024**3
@requires_resource
def test_apply_mem_limit_uses_target_when_hard_unlimited(monkeypatch): def test_apply_mem_limit_uses_target_when_hard_unlimited(monkeypatch):
captured = {} captured = {}
monkeypatch.setattr(_isolation, "_MEM_LIMIT_APPLIED", False) monkeypatch.setattr(_isolation, "_MEM_LIMIT_APPLIED", False)
@@ -147,6 +161,7 @@ def test_apply_mem_limit_uses_target_when_hard_unlimited(monkeypatch):
assert hard == resource.RLIM_INFINITY assert hard == resource.RLIM_INFINITY
@requires_resource
def test_apply_mem_limit_is_applied_once(monkeypatch): def test_apply_mem_limit_is_applied_once(monkeypatch):
calls = [] calls = []
monkeypatch.setattr(_isolation, "_MEM_LIMIT_APPLIED", False) monkeypatch.setattr(_isolation, "_MEM_LIMIT_APPLIED", False)
@@ -161,6 +176,41 @@ def test_apply_mem_limit_is_applied_once(monkeypatch):
assert len(calls) == 1 # second call is a no-op 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 ------------------------------------------------ # --- PyMuPDF processor wiring ------------------------------------------------