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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
96c0491f14
commit
fc8a4e4dfa
@@ -29,6 +29,27 @@ 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
|
||||||
|
- 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]
|
||||||
|
|||||||
@@ -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 - exercised only on Windows
|
||||||
|
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)
|
||||||
|
|||||||
@@ -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):
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ or depend on the sample files).
|
|||||||
"""
|
"""
|
||||||
|
|
||||||
import resource
|
import resource
|
||||||
|
import sys
|
||||||
|
|
||||||
import anyio
|
import anyio
|
||||||
import anyio.to_process
|
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
|
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 ------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user