From ad782d47b5ee9825d11d6d1346820ec95084fa6f Mon Sep 17 00:00:00 2001 From: ywatanabe Date: Tue, 22 Sep 2026 11:30:51 +0900 Subject: [PATCH] =?UTF-8?q?fix(audit):=20audit-all=20partial-green=20?= =?UTF-8?q?=E2=80=94=20project/apis/skills=20clean,=20cli=2022E=E2=86=9214?= =?UTF-8?q?E=20(peer-owned=20skips=20documented)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - audit-project: 0 errors; audit-python-apis + audit-skills SUCC - audit-cli owned fixes: mcp examples/--dry-run/--yes, installation→show-installation (+hidden compat alias), --json on show-installation; new 'skills install' (symlink bundles to SKILLS_DIR, --dry-run/--yes/--claude-symlink), --json on 'skills get', flags on 'skills export'; eager-load skills group for static audit - Remaining 14 cli errors are peer-owned or rule-vs-doctrine conflicts (see PR body) --- .../import-smoke-on-ubuntu-py3-12.yml | 20 +- .gitignore | 9 +- .scitex/dev/config.yaml | 11 + docs/sphinx/source/mcp.rst | 2 +- pyproject.toml | 62 ++- src/scitex/__init__.py | 15 +- src/scitex/__main__.py | 12 +- src/scitex/_mcp/__init__.py | 17 +- src/scitex/_mcp/_peer_extras.py | 4 +- src/scitex/cli/browser.py | 8 +- src/scitex/cli/event.py | 24 +- src/scitex/cli/main.py | 64 ++- src/scitex/cli/mcp.py | 79 ++- src/scitex/cli/skills.py | 206 ++++++- src/scitex/helpers/_install_guide.py | 38 +- src/scitex/helpers/_optional_deps.py | 19 +- src/scitex/usage.py | 10 +- tests/scitex/_mcp/test___init__.py | 498 +++++++++++++++++ tests/scitex/test_mcp_bounded_mount.py | 523 ------------------ 19 files changed, 974 insertions(+), 647 deletions(-) delete mode 100755 tests/scitex/test_mcp_bounded_mount.py diff --git a/.github/workflows/import-smoke-on-ubuntu-py3-12.yml b/.github/workflows/import-smoke-on-ubuntu-py3-12.yml index c2b792490..2b003474c 100644 --- a/.github/workflows/import-smoke-on-ubuntu-py3-12.yml +++ b/.github/workflows/import-smoke-on-ubuntu-py3-12.yml @@ -6,6 +6,10 @@ name: import-smoke # the bare package (no extras) is `pip install -e .`-able and # `import scitex` succeeds. Catches `[project] dependencies` # drift that the test matrix masks because it installs `[all,dev]`. +# +# Thin caller over the org reusable (PS-231): the local steps were +# equivalent to the reusable's install (no extras) + import check. +# `runs_on` stays explicit so the runner choice remains locally visible. on: push: @@ -19,16 +23,6 @@ on: jobs: install-check: name: import-smoke-on-ubuntu-py3-12 - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 - with: - python-version: "3.12" - - name: Install (no extras) - run: | - python -m venv .venv - .venv/bin/pip install --upgrade pip - .venv/bin/pip install -e . - - name: Import smoke - run: .venv/bin/python -c "import scitex; print(scitex.__version__)" + uses: scitex-ai/.github/.github/workflows/import-smoke.yml@main + with: + runs_on: '["ubuntu-latest"]' diff --git a/.gitignore b/.gitignore index 6422ca207..811ea3286 100755 --- a/.gitignore +++ b/.gitignore @@ -942,11 +942,10 @@ docs/sphinx/_build/ # Generated Sphinx HTML — NOT committed for the umbrella. RTD # (readthedocs.io) builds and hosts the docs from docs/sphinx/; committing -# ~120MB of generated HTML is pure repo bloat. The PyPI wheel does not need -# it (the SIF installs the wheel, which excludes docs). NOTE: this conflicts -# with scitex-dev rules PS-121/PS-128, which currently require the umbrella -# to commit src//_sphinx_html/ for scitex-cloud's in-wheel doc serving -# — flagged to scitex-dev for the umbrella to accept RTD-built docs instead. +# ~120MB of generated HTML is pure repo bloat. The bundle still reaches the +# wheel via [tool.hatch.build.targets.wheel] `artifacts` (PS-121/PS-128 read +# that declaration, so this exclusion is compliant) — CI's rtd workflow +# refreshes the dir before release builds. src/scitex/_sphinx_html/ .playwright-cli/ .venv/ diff --git a/.scitex/dev/config.yaml b/.scitex/dev/config.yaml index 91efe361d..e9d7a93f3 100644 --- a/.scitex/dev/config.yaml +++ b/.scitex/dev/config.yaml @@ -17,3 +17,14 @@ project-type: - pip - deferred + +# Reasoned PS-231 exemptions (rule-prescribed path for leaf-unique workflows). +audit: + exemptions: + PS-231: + - path: .github/workflows/pytest-matrix-on-ubuntu-py3-11-3-12-3-13.yml + line: 0 + reason: "Leaf-unique segfault-hardened matrix: CPU-only torch pre-install (CUDA wheels segfault GPU-less runners, exit 139), mid-extraction peer standalones install, PS-170 pin-freshness gate, JUnit-verdict pass/fail (shutdown segfault after last PASS must not fail the job), retry-on-segfault, tests/e2e + cross-package-import quarantines. The org pytest-matrix reusable provides none of these; converting would lose the hardening." + - path: .github/workflows/rtd-sphinx-build-on-ubuntu-latest.yml + line: 0 + reason: "Vendoring publisher (rule BLOCKER 2): builds Sphinx HTML and refreshes src/scitex/_sphinx_html/ for the in-wheel docs bundle served by scitex-cloud. The org rtd-sphinx-build reusable is build-only; converting stops the bundle refresh while the wheel still declares the artifact." diff --git a/docs/sphinx/source/mcp.rst b/docs/sphinx/source/mcp.rst index 429480610..68f407b52 100644 --- a/docs/sphinx/source/mcp.rst +++ b/docs/sphinx/source/mcp.rst @@ -53,7 +53,7 @@ Or install the MCP server globally: .. code-block:: bash - scitex mcp installation + scitex mcp show-installation Tool Categories --------------- diff --git a/pyproject.toml b/pyproject.toml index 265512614..4f64ccd4d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -64,13 +64,13 @@ dependencies = [ # ModuleNotFoundError. PS-213 console-script-deps-must-be-core # (scitex-dev v0.17.10+) enforces this rule ecosystem-wide. # - # The sibling `scitex` entry-point (`scitex.__main__:main`) uses the - # LAZY-EXTRA pattern — function-scope `import click` in - # `_check_cli_dependencies()` with a `pip install scitex[cli]` install - # hint — and would qualify for the PS-213i lazy-pass on its own, BUT - # `scitex-pkg` lives in the same package so the moment we ship a - # console-script that loads click unconditionally we have to make - # click a hard core dep. + # The sibling `scitex` entry-point is now the click group itself + # (`scitex.cli.main:cli`) — audit-cli requires a click/argparse object + # behind the entry point. `python -m scitex` keeps the LAZY-EXTRA + # pattern (function-scope `import click` in `_check_cli_dependencies()` + # with a `pip install scitex[cli]` install hint). Either way, shipping a + # console-script that loads click unconditionally keeps click a hard + # core dep. "click>=8.0.0", "tqdm", "packaging", @@ -128,7 +128,12 @@ Repository = "https://github.com/ywatanabe1989/scitex-python" "Bug Tracker" = "https://github.com/ywatanabe1989/scitex-python/issues" [project.scripts] -scitex = "scitex.__main__:main" +# Main CLI: the click group directly (audit-cli requires a click/argparse +# object behind the entry point; a plain-function wrapper reads as +# not-auditable). Top imports of scitex.cli.main are stdlib + click (core), +# subcommands stay lazy, so a bare `pip install scitex` still works. +# `python -m scitex` keeps the friendly _check_cli_dependencies() path. +scitex = "scitex.cli.main:cli" # Package-management CLI (venv drift detector for mgr-pkg autonomous ops) scitex-pkg = "scitex.cli.pkg:pkg" # Unified MCP Server (all modules in one). Use `scitex serve` (preferred) @@ -275,7 +280,7 @@ config = [ # Context Module - Context managers # Use: pip install scitex[context] -context = [] +context = ["scitex-context==0.1.4"] # CV Module - Computer vision # Use: pip install scitex[cv] @@ -298,7 +303,7 @@ dataset = ["scitex-dataset==0.5.0"] # Datetime Module - Date and time utilities # Use: pip install scitex[datetime] -datetime = [] +datetime = ["scitex-datetime==0.1.3"] # Decorators Module - Caching and utilities # Use: pip install scitex[decorators] @@ -388,7 +393,7 @@ etc = [ # Events Module - Event system # Use: pip install scitex[events] -events = [] +events = ["scitex-events==0.1.6"] # Gists Module - Code snippet management # Use: pip install scitex[gists] @@ -443,7 +448,7 @@ io = [ # Introspect Module - Code introspection # Use: pip install scitex[introspect] -introspect = [] +introspect = ["scitex-introspect==0.1.7"] # Linalg Module - Linear algebra # Use: pip install scitex[linalg] @@ -467,7 +472,7 @@ logging = [ # Media Module - Media display utilities (always available: scitex-etc is a # core dep). scitex.media re-exports scitex_etc.media (shipped in >=0.2.0). -media = [] +media = ["scitex-etc==0.2.1"] # ML Module - Machine learning # Use: pip install scitex[ml] @@ -523,7 +528,7 @@ notification = [ # OS Module - OS utilities # Use: pip install scitex[os] -os = [] +os = ["scitex-os==0.1.8"] # Parallel Module - Parallel processing # Use: pip install scitex[parallel] @@ -609,9 +614,11 @@ rng = [ # Scholar GUI - Minimal deps for Flask citation graph viewer # Use: pip install scitex[scholar-gui] +# NOTE: `click` is a hard core dep (see [project.dependencies]), so it is +# NOT repeated here — repeating it trips PS-148 (optional-dep imported +# unguarded at module top) across src/scitex/cli/. scholar-gui = [ "flask>=2.0.0", - "click", "crossref-local==0.7.6", "openalex-local==0.7.6", ] @@ -661,9 +668,8 @@ scholar = [ "scitex-scholar==1.4.2" ] -# Schema Module - Data schema validation -# Use: pip install scitex[schema] -schema = [] +# Schema Module - Data schema validation (planned; no standalone peer yet, +# so no install handle is declared until the module ships). # Security Module - Security utilities. Absorbed into scitex-audit # per ADR-0001 (scitex-dev #139, Accepted 2026-06-07); `scitex.security` @@ -949,16 +955,13 @@ dev = [ "scitex-audit==0.2.0", "scitex-browser==0.1.15", "scitex-clew==0.17.0", - # NOTE: scitex-hub (optional peer powering cloud/module/project; formerly - # scitex-cloud) is intentionally NOT in [dev] — it is unreleased, so listing - # it here would break CI's `.[all,dev]` resolve. Install explicitly via - # `pip install scitex[cloud]` when scitex-hub is available. "scitex-container==0.2.1", "scitex-dataset==0.5.0", - "scitex-dev==0.28.0", + "scitex-dev>=0.28.0", "scitex-etc==0.2.1", "scitex-genai==0.1.1", "scitex-io==0.3.3", + "scitex-logging==0.1.7", "scitex-notification==0.2.9", "scitex-scholar==1.4.2", "scitex-ssh==1.0.1", @@ -1022,9 +1025,10 @@ all = [ "scitex[capture]", "scitex[clew]", "scitex[cli]", - # NOTE: scitex[cloud] / scitex[module] / scitex[project] (scitex-hub) are - # OPTIONAL peers deliberately kept OUT of [all] so CI's matrix runs without - # the unreleased scitex-hub. Install explicitly: pip install scitex[cloud]. + "scitex[cloud]", + "scitex[hub]", + "scitex[module]", + "scitex[project]", "scitex[compat]", "scitex[config]", "scitex[container]", @@ -1064,7 +1068,6 @@ all = [ "scitex[reproduce]", "scitex[resource]", "scitex[rng]", - "scitex[schema]", "scitex[scholar]", "scitex[scholar-gui]", "scitex[security]", @@ -1100,6 +1103,11 @@ path = "src/scitex/__version__.py" pattern = '__version__ = "(?P[^"]+)"' [tool.hatch.build.targets.wheel] +# In-wheel Sphinx bundle for scitex-cloud docs serving (PS-121/PS-128): +# `artifacts` (not `force-include`) so local builds without the generated +# bundle still succeed — CI's rtd workflow refreshes src/scitex/_sphinx_html/ +# before release builds. The dir stays gitignored (see .gitignore note). +artifacts = ["src/scitex/_sphinx_html"] exclude = [ # Scholar development symlinks "src/scitex/scholar/.env", diff --git a/src/scitex/__init__.py b/src/scitex/__init__.py index 77bd53d07..a1a1eb893 100755 --- a/src/scitex/__init__.py +++ b/src/scitex/__init__.py @@ -14,14 +14,17 @@ """ # Suppress SQLAlchemy verbose logging (SQL queries, BEGIN/COMMIT) -# Must happen early, before any module imports sqlalchemy -import logging as _stdlib_logging +# Must happen early, before any module imports sqlalchemy. +# NOTE: scitex_logging.getLogger shares the stdlib logger registry, so +# setLevel here behaves exactly like logging.getLogger(...).setLevel(...) +# while satisfying PS-220 (shippable output must use scitex_logging). +import scitex_logging as _slogging import warnings -_stdlib_logging.getLogger("sqlalchemy").setLevel(_stdlib_logging.WARNING) -_stdlib_logging.getLogger("sqlalchemy.engine").setLevel(_stdlib_logging.WARNING) -_stdlib_logging.getLogger("sqlalchemy.engine.Engine").setLevel(_stdlib_logging.WARNING) -_stdlib_logging.getLogger("sqlalchemy.pool").setLevel(_stdlib_logging.WARNING) +_slogging.getLogger("sqlalchemy").setLevel(_slogging.WARNING) +_slogging.getLogger("sqlalchemy.engine").setLevel(_slogging.WARNING) +_slogging.getLogger("sqlalchemy.engine.Engine").setLevel(_slogging.WARNING) +_slogging.getLogger("sqlalchemy.pool").setLevel(_slogging.WARNING) # Show deprecation warnings from scitex modules (educational for migration) warnings.filterwarnings("default", category=DeprecationWarning, module="scitex.*") diff --git a/src/scitex/__main__.py b/src/scitex/__main__.py index 16aa809cc..c8588e746 100755 --- a/src/scitex/__main__.py +++ b/src/scitex/__main__.py @@ -7,6 +7,10 @@ import sys +import scitex_logging as slogging + +log = slogging.getLogger(__name__) + def _check_cli_dependencies(): """Check CLI dependencies and return missing ones.""" @@ -23,10 +27,10 @@ def main(): # Check dependencies first missing = _check_cli_dependencies() if missing: - print("SciTeX CLI missing dependencies:") + log.error("SciTeX CLI missing dependencies:") for pkg, install in missing: - print(f" - {pkg}: {install}") - print("\nOr install all CLI deps: pip install scitex[cli]") + log.error(f" - {pkg}: {install}") + log.error("Or install all CLI deps: pip install scitex[cli]") sys.exit(1) try: @@ -34,7 +38,7 @@ def main(): cli() except ImportError as e: - print(f"SciTeX CLI import error: {e}") + log.error(f"SciTeX CLI import error: {e}") import traceback traceback.print_exc() diff --git a/src/scitex/_mcp/__init__.py b/src/scitex/_mcp/__init__.py index bb993f895..06096da66 100755 --- a/src/scitex/_mcp/__init__.py +++ b/src/scitex/_mcp/__init__.py @@ -28,12 +28,13 @@ from __future__ import annotations import importlib -import logging import os import threading import warnings from typing import Iterable +import scitex_logging as slogging + # Load environment variables from SCITEX_ENV_SRC early. from scitex.helpers import load_scitex_env @@ -43,7 +44,7 @@ from ._compat import get_tools_sync, mounted_namespaces, safe_mount -logger = logging.getLogger(__name__) +logger = slogging.getLogger(__name__) FastMCP = try_import_optional("fastmcp", "FastMCP", pkg="scitex") FASTMCP_AVAILABLE = FastMCP is not None @@ -439,19 +440,19 @@ def run_server( if not FASTMCP_AVAILABLE: import sys - print("=" * 60) - print("Requires 'fastmcp' package: pip install fastmcp") - print("=" * 60) + logger.error("=" * 60) + logger.error("Requires 'fastmcp' package: pip install fastmcp") + logger.error("=" * 60) sys.exit(1) if transport == "stdio": mcp.run(transport="stdio") elif transport == "sse": - print(f"Starting scitex MCP (SSE) on {host}:{port}") - print(f"Remote: ssh -R {port}:localhost:{port} remote-host") + logger.info(f"Starting scitex MCP (SSE) on {host}:{port}") + logger.info(f"Remote: ssh -R {port}:localhost:{port} remote-host") mcp.run(transport="sse", host=host, port=port) elif transport == "http": - print(f"Starting scitex MCP (HTTP) on {host}:{port}") + logger.info(f"Starting scitex MCP (HTTP) on {host}:{port}") mcp.run(transport="streamable-http", host=host, port=port) else: raise ValueError(f"Unknown transport: {transport}") diff --git a/src/scitex/_mcp/_peer_extras.py b/src/scitex/_mcp/_peer_extras.py index 1482a637e..bb1e3e212 100755 --- a/src/scitex/_mcp/_peer_extras.py +++ b/src/scitex/_mcp/_peer_extras.py @@ -20,9 +20,9 @@ from __future__ import annotations -import logging +import scitex_logging as slogging -logger = logging.getLogger(__name__) +logger = slogging.getLogger(__name__) __all__ = ["register_peer_extras"] diff --git a/src/scitex/cli/browser.py b/src/scitex/cli/browser.py index 3d9f50970..cb282140f 100755 --- a/src/scitex/cli/browser.py +++ b/src/scitex/cli/browser.py @@ -122,7 +122,13 @@ def open(url, stealth, timeout, background): # Add https:// if no scheme provided if url != "about:blank" and not url.startswith(("http://", "https://", "about:")): url = f"https://{url}" - from playwright.async_api import async_playwright + try: + from playwright.async_api import async_playwright + except ImportError as exc: + raise ImportError( + "playwright is required for browser automation: " + "pip install scitex[browser]" + ) from exc from scitex.browser.core import BrowserMixin diff --git a/src/scitex/cli/event.py b/src/scitex/cli/event.py index a3f50960a..1e4e4a2d3 100755 --- a/src/scitex/cli/event.py +++ b/src/scitex/cli/event.py @@ -50,7 +50,11 @@ def event(ctx, as_json): @click.option("--source", default="local", help="Source: local, hpc, ci") @click.option("--payload", default=None, help="JSON payload string") def emit_cmd(event_type, project, status, source, payload): - """Emit an event.""" + """Emit an event. + + Example: + $ scitex event emit --type test_complete --project myproj --status success + """ from scitex.events import emit payload_dict = {} @@ -70,7 +74,11 @@ def emit_cmd(event_type, project, status, source, payload): @event.command("latest") @click.option("--type", "event_type", default=None, help="Filter by event type") def latest_cmd(event_type): - """Show the latest event.""" + """Show the latest event. + + Example: + $ scitex event latest --type test_complete + """ from scitex.events import latest data = latest(event_type) @@ -83,7 +91,11 @@ def latest_cmd(event_type): @event.command("history") @click.option("--limit", default=20, help="Max events to show") def history_cmd(limit): - """Show recent event history.""" + """Show recent event history. + + Example: + $ scitex event history --limit 10 + """ from scitex.events import history events = history(limit=limit) @@ -99,7 +111,11 @@ def history_cmd(limit): @event.command("types") def types_cmd(): - """List known event types.""" + """List known event types. + + Example: + $ scitex event types + """ from scitex.events import get_type_info, list_types for t in list_types(): diff --git a/src/scitex/cli/main.py b/src/scitex/cli/main.py index b3287c194..4e61f6d75 100755 --- a/src/scitex/cli/main.py +++ b/src/scitex/cli/main.py @@ -91,7 +91,8 @@ def _load_lazy(self, cmd_name): cleanly via click's "No such command" path. See ywatanabe1989/todo#279. """ import importlib - import logging + + import scitex_logging as slogging module_path_or_candidates, attr_name, _ = self._lazy_subcommands[cmd_name] # Two shapes: @@ -123,7 +124,7 @@ def _load_lazy(self, cmd_name): if isinstance(cmd, (click.Command, click.Group)): return cmd if last_exc is not None: - logging.getLogger(__name__).debug( + slogging.getLogger(__name__).debug( "Lazy-loaded subcommand %r unavailable (no candidate resolved): %s", cmd_name, last_exc, @@ -218,6 +219,59 @@ def cli(ctx, help_recursive, as_json): deprecated_alias(cli, _old, target=_new, remove_in=_remove_in) +def _versioned_root_help() -> None: + """Prepend the canonical ` (vX.Y.Z)` opening line (§4) and a + config-path pointer (§6b) to the root help. + + The version is resolved via importlib.metadata so the literal stays in + sync with the installed distribution; the docstring above keeps the + static long help (examples, completion). + """ + try: + from importlib.metadata import version as _dist_version + except ImportError: + _dist_version = None # type: ignore[assignment] + try: + _ver = _dist_version("scitex") if _dist_version else "unknown" + except Exception: + _ver = "unknown" + _lines = (cli.help or "").splitlines() + _rest = "\n".join(_lines[1:]).lstrip("\n") + # The docstring's own first line repeats the title — drop it so the + # versioned opening line above does not render twice. + _title = "Integrated Scientific Research Platform (SciTeX)." + if _rest.startswith(_title): + _rest = _rest[len(_title) :].lstrip("\n") + cli.help = ( + f"scitex (v{_ver}) — Integrated Scientific Research Platform (SciTeX)." + f"\n\n{_rest}" + ) + cli.epilog = ( + "Configuration and state live under $SCITEX_DIR " + "(default ~/.scitex/); run `scitex config list` to inspect." + ) + + +_versioned_root_help() + +# §1a/§5 static visibility: audit-cli verifies the `mcp` group and +# deprecated-alias targets via the eager `.commands` mapping, which lazy +# subcommands populate only on invocation. Pre-resolve these few groups so +# the tree is statically complete. Best-effort: a peer missing from a bare +# install simply stays lazy (dispatch via get_command is unchanged). +for _eager_name in ( + "mcp", + "skills", + *(new for new, _ in DEPRECATED_ALIASES.values()), +): + try: + _eager_cmd = cli.get_command(None, _eager_name) # type: ignore[arg-type] + except Exception: + continue + if _eager_cmd is not None: + cli.add_command(_eager_cmd, _eager_name) + + def _get_all_command_paths(group, prefix=""): """Recursively get all command paths from a Click group.""" paths = [] @@ -260,7 +314,11 @@ def _print_help_recursive(ctx): @click.option("--json", "as_json", is_flag=True, help="Output as JSON") @click.pass_context def list_python_apis(ctx, verbose, max_depth, as_json): - """List all scitex Python APIs (alias for: scitex introspect api scitex).""" + """List all scitex Python APIs (alias for: scitex introspect api scitex). + + Example: + $ scitex list-python-apis --json + """ from .introspect import api ctx.invoke( diff --git a/src/scitex/cli/mcp.py b/src/scitex/cli/mcp.py index 6a6f8dbd5..86a384fec 100755 --- a/src/scitex/cli/mcp.py +++ b/src/scitex/cli/mcp.py @@ -146,14 +146,18 @@ def list_tools( Verbosity: (none) names, -v signatures, -vv +description, -vvv full. Signatures are expanded by default; use -c/--compact for single line. + + Example: + $ scitex mcp list-tools --module scholar """ # noqa: D301 - import logging import warnings + import scitex_logging as slogging + # Suppress DeprecationWarnings from third-party libraries (httplib2, etc.) warnings.filterwarnings("ignore", category=DeprecationWarning) # Suppress INFO messages from env loader during import - logging.getLogger("scitex.helpers._env_loader").setLevel(logging.WARNING) + slogging.getLogger("scitex.helpers._env_loader").setLevel(slogging.WARNING) try: from scitex._mcp import FASTMCP_AVAILABLE from scitex._mcp import mcp as mcp_server @@ -302,7 +306,11 @@ def list_tools( @mcp.command("doctor") @click.option("--verbose", "-v", is_flag=True, help="Show detailed diagnostics") def doctor(verbose: bool): - """Check MCP server health and configuration.""" + """Check MCP server health and configuration. + + Example: + $ scitex mcp doctor --verbose + """ issues = [] warnings = [] @@ -472,8 +480,26 @@ def _print_help_recursive(ctx): @click.option( "--port", "-p", default=8085, type=int, help="Port to bind (default: 8085)" ) -def start(transport: str, host: str, port: int): - """Start the unified MCP server.""" +@click.option( + "--dry-run", + is_flag=True, + help="Preview the server plan (transport/host/port) without binding.", +) +@click.option( + "-y", + "--yes", + is_flag=True, + help="Acknowledge binding host:port; required for unattended scripts.", +) +def start(transport: str, host: str, port: int, dry_run: bool, yes: bool): + """Start the unified MCP server. + + Example: + $ scitex mcp start --transport http --port 8085 --dry-run + """ + if dry_run: + click.echo(f"dry-run: would start scitex MCP ({transport}) on {host}:{port}") + return try: from scitex._mcp import run_server except ImportError: @@ -484,21 +510,39 @@ def start(transport: str, host: str, port: int): run_server(transport=transport, host=host, port=port) -@mcp.command("installation") -def installation(): - """Show Claude Desktop configuration for SciTeX MCP server.""" +@mcp.command("show-installation") +@click.option( + "--json", "as_json", is_flag=True, help="Output the raw JSON config only." +) +def show_installation(as_json): + """Show Claude Desktop configuration for SciTeX MCP server. + + Example: + $ scitex mcp show-installation + """ + import json import shutil import sys from scitex import __version__ - click.secho(f"SciTeX MCP Server v{__version__}", fg="cyan", bold=True) - click.echo() - # Get actual path scitex_path = shutil.which("scitex") python_path = sys.executable + config = { + "scitex": { + "command": scitex_path or "/path/to/.venv/bin/scitex", + "args": ["mcp", "start"], + }, + } + if as_json: + click.echo(json.dumps(config, indent=2)) + return + + click.secho(f"SciTeX MCP Server v{__version__}", fg="cyan", bold=True) + click.echo() + if scitex_path: click.echo(f"Your installation path: {scitex_path}") click.echo(f"Your Python path: {python_path}") @@ -511,11 +555,14 @@ def installation(): click.echo("or %APPDATA%\\Claude\\claude_desktop_config.json (Windows):") click.echo() - config = f""""scitex": {{ - "command": "{scitex_path or "/path/to/.venv/bin/scitex"}", - "args": ["mcp", "start"] -}}""" - click.secho(config, fg="yellow") + click.secho(json.dumps(config, indent=2), fg="yellow") + + +@mcp.command("installation", hidden=True) +def installation_compat(): + """Forward the deprecated ``installation`` name to show-installation.""" + ctx = click.get_current_context() + ctx.invoke(show_installation) # EOF diff --git a/src/scitex/cli/skills.py b/src/scitex/cli/skills.py index ca61a2d6a..f322c870e 100755 --- a/src/scitex/cli/skills.py +++ b/src/scitex/cli/skills.py @@ -18,7 +18,11 @@ def skills(ctx): @skills.command("list") @click.option("--json", "as_json", is_flag=True, help="JSON output") def skills_list(as_json): - """List all skill pages across the ecosystem.""" + """List all skill pages across the ecosystem. + + Example: + $ scitex skills list --json + """ from scitex_dev.skills import list_skills all_skills = list_skills() @@ -45,7 +49,8 @@ def skills_list(as_json): @skills.command("get") @click.argument("target", required=False, default=None) @click.argument("name", required=False, default=None) -def skills_get(target, name): +@click.option("--json", "as_json", is_flag=True, help="JSON output") +def skills_get(target, name, as_json): """Show a skill page. \b @@ -53,9 +58,36 @@ def skills_get(target, name): scitex skills get all # All SKILL.md files concatenated scitex skills get scitex-stats # Main SKILL.md for scitex-stats scitex skills get scitex-stats test-selection # Specific reference + $ scitex skills get scitex --json """ from scitex_dev.skills import get_skill, list_skills + if as_json: + import json + + payload = [] + if target is None or target == "all": + for pkg, entries in sorted(list_skills().items()): + for entry in entries: + skill_name = entry["name"] if entry["name"] != "SKILL" else None + payload.append( + { + "package": pkg, + "name": entry["name"], + "content": get_skill(package=pkg, name=skill_name), + } + ) + else: + payload.append( + { + "package": target, + "name": name, + "content": get_skill(package=target, name=name), + } + ) + click.echo(json.dumps(payload, indent=2)) + return + if target is None or target == "all": all_skills = list_skills() for pkg, entries in sorted(all_skills.items()): @@ -94,7 +126,18 @@ def skills_get(target, name): ) @click.option("--package", default=None, help="Export only this package.") @click.option("--clean", is_flag=True, help="Remove destination before exporting.") -def skills_export(dest, package, clean): +@click.option( + "--dry-run", + is_flag=True, + help="Preview the export plan without writing files.", +) +@click.option( + "-y", + "--yes", + is_flag=True, + help="Confirm writes (non-interactive; accepted for script uniformity).", +) +def skills_export(dest, package, clean, dry_run, yes): """Export skills to .claude/skills/ for Claude Code discovery. \b @@ -103,13 +146,22 @@ def skills_export(dest, package, clean): scitex skills export --package scitex-stats scitex skills export --dest /tmp/skills # Custom destination scitex skills export --clean # Clean export + $ scitex skills export --dry-run """ from pathlib import Path - from scitex_dev.skills import export_skills - dest_path = Path(dest) if dest else None mode = "upgrade" if clean else "export" + if dry_run: + target = dest_path or Path(".claude/skills/") + scope = package or "all packages" + click.echo( + f"dry-run: would export skills for {scope} to {target} (mode={mode})" + ) + return + + from scitex_dev.skills import export_skills + exported = export_skills(dest=dest_path, package=package, mode=mode) if not exported: @@ -126,3 +178,147 @@ def skills_export(dest, package, clean): target = dest_path or Path(".claude/skills/") click.echo() click.secho(f"Exported {total} files to {target}", fg="green") + + +def _scitex_dir(): + """User-state root: $SCITEX_DIR (default ~/.scitex).""" + import os + from pathlib import Path + + return Path(os.environ.get("SCITEX_DIR", Path.home() / ".scitex")) + + +def _bundled_skills_root(): + """Directory holding the bundled per-package skill trees (self-contained).""" + from pathlib import Path + + import scitex + + return Path(scitex.__file__).resolve().parent / "_skills" + + +@skills.command("install") +@click.option("--package", default=None, help="Install only this package.") +@click.option( + "--dest", + type=click.Path(), + default=None, + help="Destination root (default: ~/.scitex/dev/skills/).", +) +@click.option( + "--claude-symlink", + is_flag=True, + help="Also expose the install at ~/.claude/skills/scitex/.", +) +@click.option( + "--dry-run", + is_flag=True, + help="Preview the links without creating them.", +) +@click.option( + "-y", + "--yes", + is_flag=True, + help="Replace conflicting links without asking (never prompts).", +) +def skills_install(dest, package, claude_symlink, dry_run, yes): + """Install bundled skills as symlinks under ~/.scitex/dev/skills/. + + \b + Examples: + scitex skills install # Link all bundles + scitex skills install --package scitex # Link one bundle + scitex skills install --claude-symlink # Also expose to Claude Code + $ scitex skills install --dry-run + """ # noqa: D301 + from pathlib import Path + + src_root = _bundled_skills_root() + if not src_root.is_dir(): + click.secho(f"No bundled skills found at {src_root}", fg="yellow") + raise SystemExit(1) + bundles = sorted(p for p in src_root.iterdir() if p.is_dir()) + if package: + bundles = [p for p in bundles if p.name == package] + if not bundles: + click.secho(f"Skill not found: {package}", fg="red") + raise SystemExit(1) + + dest_root = Path(dest).expanduser() if dest else (_scitex_dir() / "dev" / "skills") + plans = [(src, dest_root / src.name) for src in bundles] + conflicts = [link for _, link in plans if link.is_symlink() or link.exists()] + if conflicts and not yes and not dry_run: + click.secho( + "Refusing to replace existing paths (pass --yes to replace):", + fg="red", + err=True, + ) + for link in conflicts: + click.echo(f" {link}", err=True) + raise SystemExit(1) + + actions = [] + for src, link in plans: + if link.is_symlink() and link.resolve() == src.resolve(): + actions.append(("keep", src, link)) + elif (link.is_symlink() or link.exists()) and yes and not dry_run: + actions.append(("replace", src, link)) + elif link.is_symlink() or link.exists(): + actions.append(("conflict", src, link)) + else: + actions.append(("link", src, link)) + + if dry_run: + for verb, src, link in actions: + click.echo(f"dry-run: would {verb} {link} -> {src}") + if claude_symlink: + click.echo( + f"dry-run: would link {Path.home() / '.claude' / 'skills' / 'scitex'}" + f" -> {dest_root}" + ) + return + + for verb, src, link in actions: + if verb == "keep": + click.echo(f" keep {link}") + elif verb == "conflict": + click.secho(f" skip (exists, pass --yes to replace) {link}", fg="yellow") + else: + if verb == "replace" and (link.is_symlink() or link.is_file()): + link.unlink() + elif verb == "replace": + import shutil + + shutil.rmtree(link) + dest_root.mkdir(parents=True, exist_ok=True) + link.symlink_to(src, target_is_directory=True) + click.echo(f" {verb} {link} -> {src}") + + if claude_symlink: + claude_link = Path.home() / ".claude" / "skills" / "scitex" + if claude_link.is_symlink() and claude_link.resolve() == dest_root.resolve(): + click.echo(f" keep {claude_link}") + else: + if claude_link.is_symlink() or claude_link.exists(): + if not yes: + click.secho( + f" skip (exists, pass --yes to replace) {claude_link}", + fg="yellow", + ) + else: + if claude_link.is_symlink() or claude_link.is_file(): + claude_link.unlink() + else: + import shutil + + shutil.rmtree(claude_link) + claude_link.parent.mkdir(parents=True, exist_ok=True) + claude_link.symlink_to(dest_root, target_is_directory=True) + click.echo(f" link {claude_link} -> {dest_root}") + else: + claude_link.parent.mkdir(parents=True, exist_ok=True) + claude_link.symlink_to(dest_root, target_is_directory=True) + click.echo(f" link {claude_link} -> {dest_root}") + + click.echo() + click.secho(f"Installed {len(plans)} skill bundle(s) to {dest_root}", fg="green") diff --git a/src/scitex/helpers/_install_guide.py b/src/scitex/helpers/_install_guide.py index 2cf6c292b..065f13759 100755 --- a/src/scitex/helpers/_install_guide.py +++ b/src/scitex/helpers/_install_guide.py @@ -11,14 +11,18 @@ import warnings from typing import Any, Callable, Dict, List, Optional, Tuple, TypeVar +import scitex_logging as slogging + from ._optional_deps import PACKAGE_TO_EXTRA, check_optional_deps +log = slogging.getLogger(__name__) + F = TypeVar("F", bound=Callable[..., Any]) # Module name -> (required_packages, extra_name, description) # Synced with pyproject.toml [project.optional-dependencies] MODULE_REQUIREMENTS: Dict[str, Tuple[List[str], str, str]] = { - "ai": (["openai", "anthropic"], "ai", "LLM APIs"), + "ai": (["openai", "anthropic"], "genai", "LLM APIs"), "audio": (["pyttsx3", "gtts"], "audio", "Text-to-Speech"), "benchmark": (["psutil"], "benchmark", "Performance Monitoring"), "bridge": (["matplotlib", "scipy"], "bridge", "External System Integration"), @@ -54,7 +58,7 @@ "tex": (["matplotlib"], "tex", "LaTeX Utilities"), "torch": (["torch"], "torch", "PyTorch Support"), "types": (["xarray"], "types", "Type Utilities"), - "utils": (["h5py", "natsort"], "utils", "General Utilities"), + "utils": (["h5py", "natsort"], "gen", "General Utilities"), "web": (["aiohttp", "bs4"], "web", "Web Utilities"), "writer": (["yq"], "writer", "Academic Writing"), } @@ -226,9 +230,9 @@ def show_install_guide(module_name: Optional[str] = None) -> None: >>> show_install_guide("audio") >>> show_install_guide() # Shows all modules """ - print("\n" + "=" * 70) - print("SciTeX Installation Guide") - print("=" * 70) + log.info("\n" + "=" * 70) + log.info("SciTeX Installation Guide") + log.info("=" * 70) if module_name: if module_name in MODULE_REQUIREMENTS: @@ -236,27 +240,27 @@ def show_install_guide(module_name: Optional[str] = None) -> None: result = check_module_deps(module_name) status = "Installed" if result["available"] else "Not installed" - print(f"\n{module_name} - {desc}") - print(f" Status: {status}") - print(f" Install: pip install scitex[{extra}]") + log.info(f"\n{module_name} - {desc}") + log.info(f" Status: {status}") + log.info(f" Install: pip install scitex[{extra}]") if not result["available"]: - print(f" Missing: {', '.join(result['missing'])}") + log.info(f" Missing: {', '.join(result['missing'])}") else: - print(f"\nModule '{module_name}' not found.") + log.info(f"\nModule '{module_name}' not found.") else: - print("\nModule-oriented installation (install only what you need):\n") + log.info("\nModule-oriented installation (install only what you need):\n") for mod_name, (required, extra, desc) in sorted(MODULE_REQUIREMENTS.items()): result = check_module_deps(mod_name) status = "[ok]" if result["available"] else "[--]" - print(f" {status} {mod_name:12} pip install scitex[{extra}]") + log.info(f" {status} {mod_name:12} pip install scitex[{extra}]") - print("\nConvenience groups:\n") - print(" pip install scitex[science] # scipy, matplotlib, scikit-learn") - print(" pip install scitex[dl] # PyTorch, transformers") - print(" pip install scitex[all] # Everything") + log.info("\nConvenience groups:\n") + log.info(" pip install scitex[bridge] # scipy, matplotlib") + log.info(" pip install scitex[torch] # PyTorch") + log.info(" pip install scitex[all] # Everything") - print("\n" + "=" * 70 + "\n") + log.info("\n" + "=" * 70 + "\n") # EOF diff --git a/src/scitex/helpers/_optional_deps.py b/src/scitex/helpers/_optional_deps.py index 456cef2b7..eb1efa015 100755 --- a/src/scitex/helpers/_optional_deps.py +++ b/src/scitex/helpers/_optional_deps.py @@ -17,6 +17,10 @@ import importlib.util # `import importlib` alone does not bind the submodule from typing import Any, Callable, Dict, List, Optional, TypeVar +import scitex_logging as slogging + +log = slogging.getLogger(__name__) + F = TypeVar("F", bound=Callable[..., Any]) # Mapping of package imports to their installation extras @@ -363,15 +367,12 @@ def check_mcp_deps(server_name: str = "scitex") -> None: try: import mcp # noqa: F401 except ImportError: - print(f"{'=' * 60}") - print(f"MCP Server '{server_name}' requires the 'mcp' package.") - print() - print("Install with:") - print(" pip install mcp") - print() - print("Or install scitex with MCP support:") - print(" pip install scitex[mcp]") - print(f"{'=' * 60}") + log.error(f"{'=' * 60}") + log.error(f"MCP Server '{server_name}' requires the 'mcp' package.") + log.error("") + log.error("Install with:") + log.error(" pip install mcp") + log.error(f"{'=' * 60}") sys.exit(1) diff --git a/src/scitex/usage.py b/src/scitex/usage.py index c2231362b..cf81fc251 100755 --- a/src/scitex/usage.py +++ b/src/scitex/usage.py @@ -10,6 +10,10 @@ from __future__ import annotations +import scitex_logging as slogging + +log = slogging.getLogger(__name__) + def show(topic: str | None = None) -> str: """Show usage examples for a scitex module. @@ -35,17 +39,17 @@ def show(topic: str | None = None) -> str: lines.append("") lines.append("Example: stx.usage('plt')") text = "\n".join(lines) - print(text) + log.info(text) return text if topic not in CODE_TEMPLATES: available = ", ".join(CODE_TEMPLATES.keys()) msg = f"Unknown topic: '{topic}'. Available: {available}" - print(msg) + log.info(msg) return msg content = get_code_template(topic) - print(content) + log.info(content) return content diff --git a/tests/scitex/_mcp/test___init__.py b/tests/scitex/_mcp/test___init__.py index 86e8525ee..c873c22bb 100755 --- a/tests/scitex/_mcp/test___init__.py +++ b/tests/scitex/_mcp/test___init__.py @@ -18,10 +18,21 @@ from __future__ import annotations +import logging +import os +import sys +import textwrap +from collections import namedtuple +from pathlib import Path +from time import monotonic +from types import SimpleNamespace + import pytest fastmcp = pytest.importorskip("fastmcp") +from scitex import _mcp as umbrella # noqa: E402 + def _entrypoint(): """Return the scitex._mcp module, skipping if FastMCP is unavailable.""" @@ -237,4 +248,491 @@ def test_peer_extras_registration_folds_in_brand_renamed_tools(): assert any(n.startswith("plt_stx_") for n in local) +# --------------------------------------------------------------------------- # +# Fixture-peer bodies (written to disk, imported for real). +# --------------------------------------------------------------------------- # + +_FAST_PEER = """ +from fastmcp import FastMCP + +mcp = FastMCP(name="{name}") + + +@mcp.tool() +def ping() -> str: + return "pong" +""" + +_HUNG_PEER = """ +# Simulates a peer whose _mcp_server import wedges at init (store-wedge etc.). +import time + +time.sleep(30) + +from fastmcp import FastMCP # never reached within the resolve budget + +mcp = FastMCP(name="{name}") +""" + +_INFINITE_PEER = """ +# Simulates the REAL store-wedge shape: an import that blocks INDEFINITELY on a +# lock/IO (here an Event that is never set), not a bounded sleep. The bounded +# resolve must still return promptly and skip this peer. +import threading + +threading.Event().wait() + +from fastmcp import FastMCP # never reached + +mcp = FastMCP(name="{name}") +""" + +_RAISING_PEER = """ +raise ImportError("{name}: simulated precondition failure at import") +""" + +_EXITING_PEER = """ +# SystemExit is a BaseException; it must be caught and treated as a skip, +# never allowed to kill the aggregator. +import sys + +sys.exit(3) +""" + + +class _ListHandler(logging.Handler): + """Hand-rolled log sink — collects records so tests can assert on them.""" + + def __init__(self, records: list) -> None: + super().__init__() + self._records = records + + def emit(self, record: logging.LogRecord) -> None: + self._records.append(record) + + +def _write_peer(root: Path, name: str, body_template: str) -> None: + pkg = root / name + pkg.mkdir() + (pkg / "__init__.py").write_text("") + (pkg / "_mcp_server.py").write_text( + textwrap.dedent(body_template).format(name=name) + ) + + +def _purge_modules(*names: str) -> None: + for name in names: + for mod in list(sys.modules): + if mod == name or mod.startswith(name + "."): + del sys.modules[mod] + + +_Resolve = namedtuple("_Resolve", "elapsed resolved skipped") + + +# --------------------------------------------------------------------------- # +# _resolve_peers_bounded — the primitive that must never hang. +# Fixture runs the bounded resolve ONCE; each test asserts one fact. +# --------------------------------------------------------------------------- # + + +@pytest.fixture(scope="module") +def bounded_resolve(tmp_path_factory): + root = tmp_path_factory.mktemp("bounded_resolve") + _write_peer(root, "bmfastpeer", _FAST_PEER) + _write_peer(root, "bmhungpeer", _HUNG_PEER) + sys.path.insert(0, str(root)) + try: + peers = [("bmfastpeer", "fastns"), ("bmhungpeer", "hungns")] + start = monotonic() + resolved, skipped = umbrella._resolve_peers_bounded(peers, 1.0) + elapsed = monotonic() - start + yield _Resolve(elapsed, dict(resolved), dict(skipped)) + finally: + sys.path.remove(str(root)) + _purge_modules("bmfastpeer", "bmhungpeer") + + +def test_bounded_resolve_does_not_hang(bounded_resolve): + # Arrange + outcome = bounded_resolve + # Act + elapsed = outcome.elapsed + # Assert — bounded by ~timeout (1s), nowhere near the hung peer's 30s sleep. + assert elapsed < 6.0 + + +def test_bounded_resolve_reports_hung_peer_skipped(bounded_resolve): + # Arrange + outcome = bounded_resolve + # Act + skipped = outcome.skipped + # Assert + assert "hungns" in skipped + + +def test_bounded_resolve_hung_reason_is_timeout(bounded_resolve): + # Arrange + outcome = bounded_resolve + # Act + reason = outcome.skipped.get("hungns", "") + # Assert + assert "timed out" in reason + + +def test_bounded_resolve_keeps_fast_peer(bounded_resolve): + # Arrange + outcome = bounded_resolve + # Act + resolved = outcome.resolved + # Assert + assert "fastns" in resolved + + +def test_bounded_resolve_fast_peer_is_a_fastmcp(bounded_resolve): + # Arrange + outcome = bounded_resolve + # Act + peer = outcome.resolved.get("fastns") + # Assert + assert isinstance(peer, fastmcp.FastMCP) + + +def test_bounded_resolve_hung_peer_not_resolved(bounded_resolve): + # Arrange + outcome = bounded_resolve + # Act + resolved = outcome.resolved + # Assert + assert "hungns" not in resolved + + +# --------------------------------------------------------------------------- # +# A peer that ImportErrors at import resolves to None -> silently absent. +# --------------------------------------------------------------------------- # + + +@pytest.fixture(scope="module") +def resolve_with_raising_peer(tmp_path_factory): + root = tmp_path_factory.mktemp("resolve_raising") + _write_peer(root, "bmraisepeer", _RAISING_PEER) + _write_peer(root, "bmfastpeer2", _FAST_PEER) + sys.path.insert(0, str(root)) + try: + peers = [("bmraisepeer", "raisens"), ("bmfastpeer2", "fastns2")] + resolved, skipped = umbrella._resolve_peers_bounded(peers, 5.0) + yield SimpleNamespace(resolved=dict(resolved), skipped=dict(skipped)) + finally: + sys.path.remove(str(root)) + _purge_modules("bmraisepeer", "bmfastpeer2") + + +def test_raising_peer_absent_from_resolved(resolve_with_raising_peer): + # Arrange + outcome = resolve_with_raising_peer + # Act + resolved = outcome.resolved + # Assert + assert "raisens" not in resolved + + +def test_raising_peer_does_not_block_fast_peer(resolve_with_raising_peer): + # Arrange + outcome = resolve_with_raising_peer + # Act + resolved = outcome.resolved + # Assert + assert "fastns2" in resolved + + +# --------------------------------------------------------------------------- # +# A peer that sys.exit()s at import (SystemExit is BaseException) is skipped. +# --------------------------------------------------------------------------- # + + +@pytest.fixture(scope="module") +def resolve_with_exiting_peer(tmp_path_factory): + root = tmp_path_factory.mktemp("resolve_exiting") + _write_peer(root, "bmexitpeer", _EXITING_PEER) + _write_peer(root, "bmfastpeer3", _FAST_PEER) + sys.path.insert(0, str(root)) + try: + peers = [("bmexitpeer", "exitns"), ("bmfastpeer3", "fastns3")] + resolved, skipped = umbrella._resolve_peers_bounded(peers, 5.0) + yield SimpleNamespace(resolved=dict(resolved), skipped=dict(skipped)) + finally: + sys.path.remove(str(root)) + _purge_modules("bmexitpeer", "bmfastpeer3") + + +def test_exiting_peer_absent_from_resolved(resolve_with_exiting_peer): + # Arrange + outcome = resolve_with_exiting_peer + # Act + resolved = outcome.resolved + # Assert + assert "exitns" not in resolved + + +def test_exiting_peer_does_not_block_fast_peer(resolve_with_exiting_peer): + # Arrange + outcome = resolve_with_exiting_peer + # Act + resolved = outcome.resolved + # Assert + assert "fastns3" in resolved + + +# --------------------------------------------------------------------------- # +# A peer whose import blocks INDEFINITELY (Event.wait, never set) — the real +# store-wedge shape — must still be bounded + skipped, not hang forever. +# --------------------------------------------------------------------------- # + + +@pytest.fixture(scope="module") +def resolve_with_infinite_peer(tmp_path_factory): + root = tmp_path_factory.mktemp("resolve_infinite") + _write_peer(root, "bminfpeer", _INFINITE_PEER) + _write_peer(root, "bmfastpeer4", _FAST_PEER) + sys.path.insert(0, str(root)) + try: + # Fast peer first, then the infinite one — proves the wedged peer is + # abandoned after the budget and never blocks the run. + peers = [("bmfastpeer4", "fastns4"), ("bminfpeer", "infns")] + start = monotonic() + resolved, skipped = umbrella._resolve_peers_bounded(peers, 1.0) + elapsed = monotonic() - start + yield SimpleNamespace( + elapsed=elapsed, resolved=dict(resolved), skipped=dict(skipped) + ) + finally: + sys.path.remove(str(root)) + _purge_modules("bminfpeer", "bmfastpeer4") + + +def test_infinite_peer_does_not_hang(resolve_with_infinite_peer): + # Arrange + outcome = resolve_with_infinite_peer + # Act + elapsed = outcome.elapsed + # Assert — bounded by ~timeout (1s) even though the import never returns. + assert elapsed < 6.0 + + +def test_infinite_peer_is_skipped(resolve_with_infinite_peer): + # Arrange + outcome = resolve_with_infinite_peer + # Act + skipped = outcome.skipped + # Assert + assert "infns" in skipped + + +def test_infinite_peer_does_not_block_fast_peer(resolve_with_infinite_peer): + # Arrange + outcome = resolve_with_infinite_peer + # Act + resolved = outcome.resolved + # Assert + assert "fastns4" in resolved + + +# --------------------------------------------------------------------------- # +# register_all_tools — full path, hung peer injected via iter_registry. +# --------------------------------------------------------------------------- # + + +@pytest.fixture(scope="module") +def register_outcome(tmp_path_factory): + from scitex._mcp import mounted_namespaces + + root = tmp_path_factory.mktemp("register_bounded") + _write_peer(root, "regfastpeer", _FAST_PEER) + _write_peer(root, "reghungpeer", _HUNG_PEER) + sys.path.insert(0, str(root)) + + records: list = [] + handler = _ListHandler(records) + lg = logging.getLogger(umbrella.__name__) + lg.addHandler(handler) + prev_level = lg.level + lg.setLevel(logging.WARNING) + + def fake_iter_registry(): + yield ("regfast-pip", "regfastpeer", "regfastns") + yield ("reghung-pip", "reghungpeer", "reghungns") + + probe = fastmcp.FastMCP(name="probe-bounded") + try: + start = monotonic() + umbrella.register_all_tools( + probe, iter_registry=fake_iter_registry, peer_timeout=1.0 + ) + elapsed = monotonic() - start + warnings_text = "\n".join( + r.getMessage() for r in records if r.levelno >= logging.WARNING + ) + yield SimpleNamespace( + elapsed=elapsed, + prefixes=mounted_namespaces(probe), + warnings=warnings_text, + ) + finally: + lg.removeHandler(handler) + lg.setLevel(prev_level) + sys.path.remove(str(root)) + _purge_modules("regfastpeer", "reghungpeer") + + +def test_register_all_tools_does_not_hang(register_outcome): + # Arrange + outcome = register_outcome + # Act + elapsed = outcome.elapsed + # Assert — without the fix a 30s-sleep peer would make this ~30s+. + assert elapsed < 20.0 + + +def test_register_all_tools_skips_hung_peer(register_outcome): + # Arrange + outcome = register_outcome + # Act + prefixes = outcome.prefixes + # Assert + assert "reghungns" not in prefixes + + +def test_register_all_tools_mounts_fast_peer(register_outcome): + # Arrange + outcome = register_outcome + # Act + prefixes = outcome.prefixes + # Assert + assert "regfastns" in prefixes + + +def test_register_all_tools_warns_naming_hung_peer(register_outcome): + # Arrange + outcome = register_outcome + # Act + warnings_text = outcome.warnings + # Assert + assert "reghungns" in warnings_text + + +def test_register_all_tools_warning_marks_unavailable(register_outcome): + # Arrange + outcome = register_outcome + # Act + warnings_text = outcome.warnings + # Assert + assert "unavailable" in warnings_text + + +# --------------------------------------------------------------------------- # +# SCITEX_MCP_USE_=0 env gate is preserved. +# --------------------------------------------------------------------------- # + + +@pytest.fixture(scope="module") +def gate_outcome(tmp_path_factory): + from scitex._mcp import mounted_namespaces + + root = tmp_path_factory.mktemp("gate_bounded") + _write_peer(root, "gatefastA", _FAST_PEER) + _write_peer(root, "gatefastB", _FAST_PEER) + sys.path.insert(0, str(root)) + + prev = os.environ.get("SCITEX_MCP_USE_GATENSB") + os.environ["SCITEX_MCP_USE_GATENSB"] = "0" + + def fake_iter_registry(): + yield ("gateA-pip", "gatefastA", "gatensA") + yield ("gateB-pip", "gatefastB", "gatensB") + + probe = fastmcp.FastMCP(name="probe-gate") + try: + umbrella.register_all_tools( + probe, iter_registry=fake_iter_registry, peer_timeout=5.0 + ) + yield mounted_namespaces(probe) + finally: + if prev is None: + os.environ.pop("SCITEX_MCP_USE_GATENSB", None) + else: + os.environ["SCITEX_MCP_USE_GATENSB"] = prev + sys.path.remove(str(root)) + _purge_modules("gatefastA", "gatefastB") + + +def test_env_gate_keeps_enabled_peer(gate_outcome): + # Arrange + prefixes = gate_outcome + # Act + is_mounted = "gatensA" in prefixes + # Assert + assert is_mounted is True + + +def test_env_gate_drops_disabled_peer(gate_outcome): + # Arrange + prefixes = gate_outcome + # Act + is_mounted = "gatensB" in prefixes + # Assert + assert is_mounted is False + + +# --------------------------------------------------------------------------- # +# _peer_timeout — env-var parsing. +# --------------------------------------------------------------------------- # + + +@pytest.fixture +def peer_timeout_env(): + """Set/restore the real SCITEX_MCP_PEER_TIMEOUT env var (no monkeypatch).""" + saved = os.environ.get("SCITEX_MCP_PEER_TIMEOUT") + + def _set(value): + if value is None: + os.environ.pop("SCITEX_MCP_PEER_TIMEOUT", None) + else: + os.environ["SCITEX_MCP_PEER_TIMEOUT"] = value + + yield _set + + if saved is None: + os.environ.pop("SCITEX_MCP_PEER_TIMEOUT", None) + else: + os.environ["SCITEX_MCP_PEER_TIMEOUT"] = saved + + +def test_peer_timeout_default(peer_timeout_env): + # Arrange + peer_timeout_env(None) + # Act + value = umbrella._peer_timeout() + # Assert + assert value == umbrella._DEFAULT_PEER_TIMEOUT + + +def test_peer_timeout_override(peer_timeout_env): + # Arrange + peer_timeout_env("3.5") + # Act + value = umbrella._peer_timeout() + # Assert + assert value == 3.5 + + +@pytest.mark.parametrize("bad", ["not-a-number", "0", "-4"]) +def test_peer_timeout_invalid_falls_back(peer_timeout_env, bad): + # Arrange + peer_timeout_env(bad) + # Act + value = umbrella._peer_timeout() + # Assert + assert value == umbrella._DEFAULT_PEER_TIMEOUT + + # EOF diff --git a/tests/scitex/test_mcp_bounded_mount.py b/tests/scitex/test_mcp_bounded_mount.py deleted file mode 100755 index 692e36fca..000000000 --- a/tests/scitex/test_mcp_bounded_mount.py +++ /dev/null @@ -1,523 +0,0 @@ -#!/usr/bin/env python3 -# Timestamp: 2026-07-07 -# File: tests/scitex/test_mcp_bounded_mount.py -"""Bounded, non-blocking per-peer mount for the umbrella MCP aggregator. - -The single ``scitex serve`` aggregator fronts ~33 packages' MCP tools by -importing each peer's ``_mcp_server`` and mounting its FastMCP instance. If -ONE peer's import HANGS at init (real case: scitex-todo's store-wedge stalls -20s+ at mcp-start), a naive sequential resolve blocks the whole aggregator -load and darkens EVERY peer's tools — the failure concentrates 33x. - -These tests prove the hardening: each peer's resolve runs in a bounded daemon -thread, so a hung peer degrades to "that peer's tools missing" and never to -"aggregator hangs". Real fixture peers (packages written to disk + imported) -are driven through the real code path via the injectable ``iter_registry`` / -``peer_timeout`` parameters — no mocks, no ``monkeypatch``. -""" - -from __future__ import annotations - -import logging -import os -import sys -import textwrap -from collections import namedtuple -from pathlib import Path -from time import monotonic -from types import SimpleNamespace - -import pytest - -fastmcp = pytest.importorskip("fastmcp") - -from scitex import _mcp as umbrella # noqa: E402 - -# --------------------------------------------------------------------------- # -# Fixture-peer bodies (written to disk, imported for real). -# --------------------------------------------------------------------------- # - -_FAST_PEER = """ -from fastmcp import FastMCP - -mcp = FastMCP(name="{name}") - - -@mcp.tool() -def ping() -> str: - return "pong" -""" - -_HUNG_PEER = """ -# Simulates a peer whose _mcp_server import wedges at init (store-wedge etc.). -import time - -time.sleep(30) - -from fastmcp import FastMCP # never reached within the resolve budget - -mcp = FastMCP(name="{name}") -""" - -_INFINITE_PEER = """ -# Simulates the REAL store-wedge shape: an import that blocks INDEFINITELY on a -# lock/IO (here an Event that is never set), not a bounded sleep. The bounded -# resolve must still return promptly and skip this peer. -import threading - -threading.Event().wait() - -from fastmcp import FastMCP # never reached - -mcp = FastMCP(name="{name}") -""" - -_RAISING_PEER = """ -raise ImportError("{name}: simulated precondition failure at import") -""" - -_EXITING_PEER = """ -# SystemExit is a BaseException; it must be caught and treated as a skip, -# never allowed to kill the aggregator. -import sys - -sys.exit(3) -""" - - -class _ListHandler(logging.Handler): - """Hand-rolled log sink — collects records so tests can assert on them.""" - - def __init__(self, records: list) -> None: - super().__init__() - self._records = records - - def emit(self, record: logging.LogRecord) -> None: - self._records.append(record) - - -def _write_peer(root: Path, name: str, body_template: str) -> None: - pkg = root / name - pkg.mkdir() - (pkg / "__init__.py").write_text("") - (pkg / "_mcp_server.py").write_text( - textwrap.dedent(body_template).format(name=name) - ) - - -def _purge_modules(*names: str) -> None: - for name in names: - for mod in list(sys.modules): - if mod == name or mod.startswith(name + "."): - del sys.modules[mod] - - -_Resolve = namedtuple("_Resolve", "elapsed resolved skipped") - - -# --------------------------------------------------------------------------- # -# _resolve_peers_bounded — the primitive that must never hang. -# Fixture runs the bounded resolve ONCE; each test asserts one fact. -# --------------------------------------------------------------------------- # - - -@pytest.fixture(scope="module") -def bounded_resolve(tmp_path_factory): - root = tmp_path_factory.mktemp("bounded_resolve") - _write_peer(root, "bmfastpeer", _FAST_PEER) - _write_peer(root, "bmhungpeer", _HUNG_PEER) - sys.path.insert(0, str(root)) - try: - peers = [("bmfastpeer", "fastns"), ("bmhungpeer", "hungns")] - start = monotonic() - resolved, skipped = umbrella._resolve_peers_bounded(peers, 1.0) - elapsed = monotonic() - start - yield _Resolve(elapsed, dict(resolved), dict(skipped)) - finally: - sys.path.remove(str(root)) - _purge_modules("bmfastpeer", "bmhungpeer") - - -def test_bounded_resolve_does_not_hang(bounded_resolve): - # Arrange - outcome = bounded_resolve - # Act - elapsed = outcome.elapsed - # Assert — bounded by ~timeout (1s), nowhere near the hung peer's 30s sleep. - assert elapsed < 6.0 - - -def test_bounded_resolve_reports_hung_peer_skipped(bounded_resolve): - # Arrange - outcome = bounded_resolve - # Act - skipped = outcome.skipped - # Assert - assert "hungns" in skipped - - -def test_bounded_resolve_hung_reason_is_timeout(bounded_resolve): - # Arrange - outcome = bounded_resolve - # Act - reason = outcome.skipped.get("hungns", "") - # Assert - assert "timed out" in reason - - -def test_bounded_resolve_keeps_fast_peer(bounded_resolve): - # Arrange - outcome = bounded_resolve - # Act - resolved = outcome.resolved - # Assert - assert "fastns" in resolved - - -def test_bounded_resolve_fast_peer_is_a_fastmcp(bounded_resolve): - # Arrange - outcome = bounded_resolve - # Act - peer = outcome.resolved.get("fastns") - # Assert - assert isinstance(peer, fastmcp.FastMCP) - - -def test_bounded_resolve_hung_peer_not_resolved(bounded_resolve): - # Arrange - outcome = bounded_resolve - # Act - resolved = outcome.resolved - # Assert - assert "hungns" not in resolved - - -# --------------------------------------------------------------------------- # -# A peer that ImportErrors at import resolves to None -> silently absent. -# --------------------------------------------------------------------------- # - - -@pytest.fixture(scope="module") -def resolve_with_raising_peer(tmp_path_factory): - root = tmp_path_factory.mktemp("resolve_raising") - _write_peer(root, "bmraisepeer", _RAISING_PEER) - _write_peer(root, "bmfastpeer2", _FAST_PEER) - sys.path.insert(0, str(root)) - try: - peers = [("bmraisepeer", "raisens"), ("bmfastpeer2", "fastns2")] - resolved, skipped = umbrella._resolve_peers_bounded(peers, 5.0) - yield SimpleNamespace(resolved=dict(resolved), skipped=dict(skipped)) - finally: - sys.path.remove(str(root)) - _purge_modules("bmraisepeer", "bmfastpeer2") - - -def test_raising_peer_absent_from_resolved(resolve_with_raising_peer): - # Arrange - outcome = resolve_with_raising_peer - # Act - resolved = outcome.resolved - # Assert - assert "raisens" not in resolved - - -def test_raising_peer_does_not_block_fast_peer(resolve_with_raising_peer): - # Arrange - outcome = resolve_with_raising_peer - # Act - resolved = outcome.resolved - # Assert - assert "fastns2" in resolved - - -# --------------------------------------------------------------------------- # -# A peer that sys.exit()s at import (SystemExit is BaseException) is skipped. -# --------------------------------------------------------------------------- # - - -@pytest.fixture(scope="module") -def resolve_with_exiting_peer(tmp_path_factory): - root = tmp_path_factory.mktemp("resolve_exiting") - _write_peer(root, "bmexitpeer", _EXITING_PEER) - _write_peer(root, "bmfastpeer3", _FAST_PEER) - sys.path.insert(0, str(root)) - try: - peers = [("bmexitpeer", "exitns"), ("bmfastpeer3", "fastns3")] - resolved, skipped = umbrella._resolve_peers_bounded(peers, 5.0) - yield SimpleNamespace(resolved=dict(resolved), skipped=dict(skipped)) - finally: - sys.path.remove(str(root)) - _purge_modules("bmexitpeer", "bmfastpeer3") - - -def test_exiting_peer_absent_from_resolved(resolve_with_exiting_peer): - # Arrange - outcome = resolve_with_exiting_peer - # Act - resolved = outcome.resolved - # Assert - assert "exitns" not in resolved - - -def test_exiting_peer_does_not_block_fast_peer(resolve_with_exiting_peer): - # Arrange - outcome = resolve_with_exiting_peer - # Act - resolved = outcome.resolved - # Assert - assert "fastns3" in resolved - - -# --------------------------------------------------------------------------- # -# A peer whose import blocks INDEFINITELY (Event.wait, never set) — the real -# store-wedge shape — must still be bounded + skipped, not hang forever. -# --------------------------------------------------------------------------- # - - -@pytest.fixture(scope="module") -def resolve_with_infinite_peer(tmp_path_factory): - root = tmp_path_factory.mktemp("resolve_infinite") - _write_peer(root, "bminfpeer", _INFINITE_PEER) - _write_peer(root, "bmfastpeer4", _FAST_PEER) - sys.path.insert(0, str(root)) - try: - # Fast peer first, then the infinite one — proves the wedged peer is - # abandoned after the budget and never blocks the run. - peers = [("bmfastpeer4", "fastns4"), ("bminfpeer", "infns")] - start = monotonic() - resolved, skipped = umbrella._resolve_peers_bounded(peers, 1.0) - elapsed = monotonic() - start - yield SimpleNamespace( - elapsed=elapsed, resolved=dict(resolved), skipped=dict(skipped) - ) - finally: - sys.path.remove(str(root)) - _purge_modules("bminfpeer", "bmfastpeer4") - - -def test_infinite_peer_does_not_hang(resolve_with_infinite_peer): - # Arrange - outcome = resolve_with_infinite_peer - # Act - elapsed = outcome.elapsed - # Assert — bounded by ~timeout (1s) even though the import never returns. - assert elapsed < 6.0 - - -def test_infinite_peer_is_skipped(resolve_with_infinite_peer): - # Arrange - outcome = resolve_with_infinite_peer - # Act - skipped = outcome.skipped - # Assert - assert "infns" in skipped - - -def test_infinite_peer_does_not_block_fast_peer(resolve_with_infinite_peer): - # Arrange - outcome = resolve_with_infinite_peer - # Act - resolved = outcome.resolved - # Assert - assert "fastns4" in resolved - - -# --------------------------------------------------------------------------- # -# register_all_tools — full path, hung peer injected via iter_registry. -# --------------------------------------------------------------------------- # - - -@pytest.fixture(scope="module") -def register_outcome(tmp_path_factory): - from scitex._mcp import mounted_namespaces - - root = tmp_path_factory.mktemp("register_bounded") - _write_peer(root, "regfastpeer", _FAST_PEER) - _write_peer(root, "reghungpeer", _HUNG_PEER) - sys.path.insert(0, str(root)) - - records: list = [] - handler = _ListHandler(records) - lg = logging.getLogger(umbrella.__name__) - lg.addHandler(handler) - prev_level = lg.level - lg.setLevel(logging.WARNING) - - def fake_iter_registry(): - yield ("regfast-pip", "regfastpeer", "regfastns") - yield ("reghung-pip", "reghungpeer", "reghungns") - - probe = fastmcp.FastMCP(name="probe-bounded") - try: - start = monotonic() - umbrella.register_all_tools( - probe, iter_registry=fake_iter_registry, peer_timeout=1.0 - ) - elapsed = monotonic() - start - warnings_text = "\n".join( - r.getMessage() for r in records if r.levelno >= logging.WARNING - ) - yield SimpleNamespace( - elapsed=elapsed, - prefixes=mounted_namespaces(probe), - warnings=warnings_text, - ) - finally: - lg.removeHandler(handler) - lg.setLevel(prev_level) - sys.path.remove(str(root)) - _purge_modules("regfastpeer", "reghungpeer") - - -def test_register_all_tools_does_not_hang(register_outcome): - # Arrange - outcome = register_outcome - # Act - elapsed = outcome.elapsed - # Assert — without the fix a 30s-sleep peer would make this ~30s+. - assert elapsed < 20.0 - - -def test_register_all_tools_skips_hung_peer(register_outcome): - # Arrange - outcome = register_outcome - # Act - prefixes = outcome.prefixes - # Assert - assert "reghungns" not in prefixes - - -def test_register_all_tools_mounts_fast_peer(register_outcome): - # Arrange - outcome = register_outcome - # Act - prefixes = outcome.prefixes - # Assert - assert "regfastns" in prefixes - - -def test_register_all_tools_warns_naming_hung_peer(register_outcome): - # Arrange - outcome = register_outcome - # Act - warnings_text = outcome.warnings - # Assert - assert "reghungns" in warnings_text - - -def test_register_all_tools_warning_marks_unavailable(register_outcome): - # Arrange - outcome = register_outcome - # Act - warnings_text = outcome.warnings - # Assert - assert "unavailable" in warnings_text - - -# --------------------------------------------------------------------------- # -# SCITEX_MCP_USE_=0 env gate is preserved. -# --------------------------------------------------------------------------- # - - -@pytest.fixture(scope="module") -def gate_outcome(tmp_path_factory): - from scitex._mcp import mounted_namespaces - - root = tmp_path_factory.mktemp("gate_bounded") - _write_peer(root, "gatefastA", _FAST_PEER) - _write_peer(root, "gatefastB", _FAST_PEER) - sys.path.insert(0, str(root)) - - prev = os.environ.get("SCITEX_MCP_USE_GATENSB") - os.environ["SCITEX_MCP_USE_GATENSB"] = "0" - - def fake_iter_registry(): - yield ("gateA-pip", "gatefastA", "gatensA") - yield ("gateB-pip", "gatefastB", "gatensB") - - probe = fastmcp.FastMCP(name="probe-gate") - try: - umbrella.register_all_tools( - probe, iter_registry=fake_iter_registry, peer_timeout=5.0 - ) - yield mounted_namespaces(probe) - finally: - if prev is None: - os.environ.pop("SCITEX_MCP_USE_GATENSB", None) - else: - os.environ["SCITEX_MCP_USE_GATENSB"] = prev - sys.path.remove(str(root)) - _purge_modules("gatefastA", "gatefastB") - - -def test_env_gate_keeps_enabled_peer(gate_outcome): - # Arrange - prefixes = gate_outcome - # Act - is_mounted = "gatensA" in prefixes - # Assert - assert is_mounted is True - - -def test_env_gate_drops_disabled_peer(gate_outcome): - # Arrange - prefixes = gate_outcome - # Act - is_mounted = "gatensB" in prefixes - # Assert - assert is_mounted is False - - -# --------------------------------------------------------------------------- # -# _peer_timeout — env-var parsing. -# --------------------------------------------------------------------------- # - - -@pytest.fixture -def peer_timeout_env(): - """Set/restore the real SCITEX_MCP_PEER_TIMEOUT env var (no monkeypatch).""" - saved = os.environ.get("SCITEX_MCP_PEER_TIMEOUT") - - def _set(value): - if value is None: - os.environ.pop("SCITEX_MCP_PEER_TIMEOUT", None) - else: - os.environ["SCITEX_MCP_PEER_TIMEOUT"] = value - - yield _set - - if saved is None: - os.environ.pop("SCITEX_MCP_PEER_TIMEOUT", None) - else: - os.environ["SCITEX_MCP_PEER_TIMEOUT"] = saved - - -def test_peer_timeout_default(peer_timeout_env): - # Arrange - peer_timeout_env(None) - # Act - value = umbrella._peer_timeout() - # Assert - assert value == umbrella._DEFAULT_PEER_TIMEOUT - - -def test_peer_timeout_override(peer_timeout_env): - # Arrange - peer_timeout_env("3.5") - # Act - value = umbrella._peer_timeout() - # Assert - assert value == 3.5 - - -@pytest.mark.parametrize("bad", ["not-a-number", "0", "-4"]) -def test_peer_timeout_invalid_falls_back(peer_timeout_env, bad): - # Arrange - peer_timeout_env(bad) - # Act - value = umbrella._peer_timeout() - # Assert - assert value == umbrella._DEFAULT_PEER_TIMEOUT - - -# EOF