diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ed5c2ed..5bcf167 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -152,7 +152,11 @@ jobs: images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} tags: | type=ref,event=branch - type=sha,prefix={{branch}}- + type=ref,event=pr + # {{branch}} is only populated for branch pushes; on a pull_request + # it is empty and the tag becomes "-", which docker rejects. + type=sha,prefix={{branch}}-,enable=${{ github.event_name != 'pull_request' }} + type=sha,prefix=pr-,enable=${{ github.event_name == 'pull_request' }} type=raw,value=latest,enable={{is_default_branch}} - name: Extract metadata for production image @@ -163,7 +167,9 @@ jobs: images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} tags: | type=ref,event=branch,suffix=-production - type=sha,prefix={{branch}}-production- + type=ref,event=pr,suffix=-production + type=sha,prefix={{branch}}-production-,enable=${{ github.event_name != 'pull_request' }} + type=sha,prefix=pr-production-,enable=${{ github.event_name == 'pull_request' }} type=raw,value=production-latest,enable={{is_default_branch}} - name: Build and push development Docker image diff --git a/CHANGELOG.md b/CHANGELOG.md index 722a2de..4092de9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,12 +7,84 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.10.0] - 2026-07-27 + +### Security +- **`packages` was an injection vector**: `execute_r_analysis` interpolated + caller-supplied strings straight into `library(...)` lines, and + `validate_r_code` never saw them. A value like + `"stats); system('...'); library(utils"` executed arbitrary R, bypassing the + package allowlist, the dangerous-pattern block, and every approval category. + Entries are now required to be bare R identifiers and are checked against the + allowlist (or session approval) before use. +- **File writes are confined to the VFS roots**: `write_csv`/`write_excel`/ + `write_json` and the six visualization tools validate their `file_path` + before handing it to R, via the new `VFS.validate_write_path()`. Previously + `VFS.write_file` had no production callers at all, so the VFS enforced + nothing. Fails open when no VFS is configured, so embedders are unaffected. +- **`approve_operation` no longer disables read-only globally**: it grants + write access to the requested directory via `VFS.grant_write()` instead of + setting `vfs.read_only = False` for the whole process. + ### Added +- `approve_operation` and `approve_r_package` are registered and reachable. + They were decorated with `@tool` but never passed to `register_tool_functions`, + so `execute_r_analysis`'s "call approve_operation" instruction pointed at a + tool no client could see. +- `RConfig.max_concurrent` (env `RMCP_R_MAX_CONCURRENT`) to bound concurrent R + subprocesses; previously a hard-coded `4`. - Guarded dependabot automation: weekly grouped patch/minor updates for uv and GitHub Actions with a 7-day release cooldown; auto-approve + squash auto-merge for patch/minor (majors stay open for review); `main` ruleset requires CI checks for PR merges (admins bypass for direct pushes) +### Fixed +- **`rmcp serve` corrupted the stdio JSON-RPC stream**: it never configured + structlog, whose default factory writes to stdout. +- **R timeouts orphaned the subprocess**: `asyncio.wait_for` sat inside an + `asyncio.TaskGroup`, so the timeout surfaced as a `BaseExceptionGroup` that + the sibling `except TimeoutError` could not catch and `proc.kill()` never + ran. The timeout now wraps the whole group, and the message reports the + configured value rather than a hard-coded 120 seconds. +- **Concurrent R calls failed after the first event loop**: the module-level + `asyncio.Semaphore` bound itself to whichever loop first made a caller wait, + then raised "bound to a different event loop" everywhere else. Now one + limiter per running loop. +- **`load_example` masked every error it hit**: its fallback dict omitted + required output properties, so real failures surfaced as + `'statistics' is a required property`. It re-raises instead. +- `execute_r_analysis`'s `timeout_seconds` was declared and ignored. +- CORS was pinned to `*`: the configured `cors_origins` were never forwarded, + and their `http://localhost:*` wildcards are not matchable by Starlette's + `allow_origins` anyway. They are now compiled to `allow_origin_regex`. +- Approvals are stored on `LifespanState` rather than the per-request + `Context`, so they survive to the next request as documented. +- Oversized tool results are capped at 32 retained entries; an expired + `rmcp://data/{id}` link now explains it expired instead of raising `KeyError`. + +### Changed +- **`tools/list` shrank ~66%**, from 103,939 to ~35,500 characters (~26k to + ~9k tokens), while gaining two tools. `outputSchema` is no longer advertised + (it was 54% of the payload; results are still validated against it + server-side), and the 46 templated four-sentence descriptions are now + one-liners. +- The `initialize` instructions blob went from 2,789 to 922 characters and + stopped duplicating the tool catalogue. It was also defined twice verbatim. +- Tests that swallowed their own assertions now fail properly; several were + passing regardless of outcome. Fixing them surfaced a real wrong assertion + in `test_logistic_regression_separation_warning`. + +### Removed +- `write_csv`'s `append` parameter: R's `write.csv` refuses it + ("attempt to set 'append' ignored") and truncates, so the schema advertised + behaviour that never happened and silently lost data. +- ~1,700 lines of advertised-but-unwired subsystems: `package_tiers.py`, + `package_security.py` (the documented "4-tier security system", which was + never wired into any execution path), `discovery.py`, and `r_session.py`. +- Stale documentation: two environment variables and an approval API that do + not exist, a "Zero Skipped Tests" claim, and hard-coded tool counts that + disagreed with each other in four places. + ## [0.9.1] - 2026-07-22 ### Fixed diff --git a/CLAUDE.md b/CLAUDE.md index 4383412..850d4ee 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -33,7 +33,7 @@ rmcp start # Full R integration available **For production deployment (optimized):** ```bash docker build --target production -t rmcp-production . # Multi-stage optimized build -docker run -p 8000:8000 rmcp-production rmcp http # Production HTTP server +docker run -p 8000:8000 rmcp-production rmcp serve-http # Production HTTP server ``` ### Testing (Hybrid Strategy) @@ -76,8 +76,8 @@ uv run sphinx-autogen docs/**/*.rst # Generate autosummary stubs - **Transport Layer** (`rmcp/transport/`): Official MCP SDK transports (stdio + Streamable HTTP) via `rmcp/transport/sdk.py`; protocol bridge in `rmcp/core/sdk_adapter.py` - **Core Server** (`rmcp/core/`): MCPServer, Context management, JSON-RPC protocol - **Registries** (`rmcp/registries/`): Dynamic registration for tools, resources, prompts -- **Tools** (`rmcp/tools/`): 52 statistical analysis tools across 11 categories -- **Comprehensive Package Whitelist**: 429 R packages from CRAN task views with tiered security +- **Tools** (`rmcp/tools/`): 54 tools across 11 categories +- **Package Allowlist**: 429 R packages from CRAN task views - **R Integration** (`rmcp/r_integration.py`): Python-R bridge via subprocess + JSON ### Adding New Statistical Tools @@ -118,17 +118,17 @@ uv run sphinx-autogen docs/**/*.rst # Generate autosummary stubs - `test_deployment_scenarios.py`: Docker environment validation, production builds, multi-platform testing **Development Utilities** -- **`scripts/testing/run_comprehensive_tests.py`**: Comprehensive test runner for development (tests all 52 tools with real R) +- **`scripts/testing/run_comprehensive_tests.py`**: Comprehensive test runner for development (exercises every tool with real R) **Complete Test Coverage**: Docker environment includes **all 240+ tests** with comprehensive coverage across all components: -- ✅ **R Integration**: 52 statistical tools with real R execution +- ✅ **R Integration**: statistical tools exercised against real R - ✅ **HTTP Transport**: MCP SDK Streamable HTTP, uvicorn, bearer auth, session management - ✅ **Core MCP Protocol**: JSON-RPC 2.0, tool calls, capabilities, error handling - ✅ **Configuration System**: Environment variables, config files, hierarchical loading - ✅ **Production Deployment**: Multi-stage Docker builds, security validation, size optimization - ✅ **Scalability**: Concurrent request handling, load testing, performance validation - ✅ **Cross-platform**: Multi-architecture support, numerical consistency, platform compatibility -- ✅ **Zero Skipped Tests**: All tests execute successfully with no dependency-related skips +- ⏭️ **Dependency-gated skips**: Docker and Claude Desktop scenarios skip when those aren't present **Strategy**: Tests progress from basic functionality → protocol compliance → component integration → complete scenarios. Each tier builds on the previous, ensuring fast feedback for basic issues while providing comprehensive validation for complex workflows. @@ -372,7 +372,7 @@ OPERATION_CATEGORIES = { 2. **User Notification**: Clear description of operation and security implications shown 3. **Approval Decision**: User accepts/denies with session-wide persistence 4. **Execution**: Approved operations proceed, denied operations are blocked -5. **Session Memory**: Decisions persist for the current session to avoid repetitive prompts +5. **Session Memory**: Decisions are stored on `LifespanState`, so they persist across requests for the life of the server process ### **Security Features** @@ -421,32 +421,29 @@ OPERATION_CATEGORIES["database_operations"] = { ```python # Unit tests in tests/unit/tools/test_operation_approval.py def test_approval_required(): - context = create_test_context() - code = 'ggsave("test.png")' + context = Context.create("test", "test", LifespanState()) - # Should require approval - result = await validate_r_code(context, code) - assert result["requires_approval"] is True - assert result["operation_info"]["category"] == "file_operations" + # validate_r_code is synchronous and returns (is_safe, error) + is_safe, error = validate_r_code('ggsave("test.png")', context) + assert not is_safe + assert error == "OPERATION_APPROVAL_NEEDED:file_operations:ggsave" ``` -### **Configuration Options** +### **Approval State** -**Environment Variables:** -```bash -# Disable approval system (for automation/testing) -export RMCP_DISABLE_OPERATION_APPROVAL=true - -# Set default approval for specific categories -export RMCP_AUTO_APPROVE_FILE_OPERATIONS=true -``` +Approvals are recorded on `LifespanState` (`rmcp/core/context.py`), not on the +per-request `Context` — a fresh `Context` is built for every MCP request, so +anything stored there would be forgotten before the next call: -**Session Configuration:** ```python -# Programmatic approval (for API integrations) -context.approval_state.approve_category("file_operations") +context.lifespan.approved_operations # {category: {operation: metadata}} +context.lifespan.approved_packages # set[str] ``` +They persist for the life of the server process. There is no environment +variable to disable the approval system; the tools `approve_operation` and +`approve_r_package` are the only way to grant consent. + This system balances security with usability, ensuring sensitive operations require explicit user consent while maintaining smooth workflows for approved operations. ## Comprehensive R Package Management @@ -465,36 +462,16 @@ RMCP v0.5.1 introduces a **systematic, evidence-based R package whitelist** with **Additional Categories**: Spatial analysis, optimization, meta-analysis, clinical trials, robust statistics, missing data, NLP, experimental design, network analysis. -### **Tiered Security System** - -**4-Tier Permission Model** balances functionality with security: - -1. **Tier 1 - Auto-Approved (52 packages)**: Core statistical packages - - Base R, essential tidyverse, fundamental statistical packages - - Examples: `ggplot2`, `dplyr`, `MASS`, `survival`, `Matrix` - -2. **Tier 2 - User Approval (56 packages)**: Extended functionality - - Popular ML, econometrics, time series packages - - Examples: `caret`, `randomForest`, `forecast`, `AER`, `rstan` - -3. **Tier 3 - Admin Approval (75 packages)**: Specialized/development - - Advanced ML, development tools, web packages - - Examples: `xgboost`, `devtools`, `httr`, `tensorflow` - -4. **Tier 4 - Blocked (26 packages)**: Security risks - - System access, external dependencies, compilation tools - - Examples: `rJava`, `RMySQL`, `processx`, `system` calls - -### **Security Assessment** +### **Enforcement** -**Risk Categorization** by security impact: -- **System Access**: 4 packages (R.utils, unix, etc.) -- **Network Access**: 8 packages (curl, httr, etc.) -- **File Operations**: 8 packages (readr, openxlsx, etc.) -- **Code Execution**: 8 packages (Rcpp, devtools, etc.) -- **External Dependencies**: 7 packages (rJava, database drivers, etc.) +Package access is a single flat allowlist: a package is either in `ALLOWED_R_PACKAGES` +or it is refused. A refused package can be permitted for the session with +`approve_r_package`. -**Results**: 413 low-risk packages (96.3%), 4 medium-risk, 12 high-risk +Enforcement is regex over the R source text, so it catches honest mistakes but not +a determined caller — `do.call("library", list(pkg))` and `eval(parse(text=...))` +both evade it. Treat the allowlist as a guardrail, not a sandbox boundary; the real +isolation is the process and filesystem the server runs under. ### **Usage Examples** diff --git a/README.md b/README.md index 5b60cf4..599e32a 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ [![Documentation](https://github.com/finite-sample/rmcp/actions/workflows/docs.yml/badge.svg)](https://finite-sample.github.io/rmcp/) [![License](https://img.shields.io/github/license/finite-sample/rmcp)](https://github.com/finite-sample/rmcp/blob/main/LICENSE) -**Turn conversations into comprehensive statistical analysis** - A Model Context Protocol (MCP) server with **52 statistical analysis tools** across 11 categories and **429 R packages** from systematic CRAN task views. RMCP enables AI assistants to perform sophisticated statistical modeling, econometric analysis, machine learning, time series analysis, and data science tasks through natural conversation. +**Turn conversations into comprehensive statistical analysis** - A Model Context Protocol (MCP) server with **54 tools** across 11 categories and **429 R packages** from systematic CRAN task views. RMCP enables AI assistants to perform sophisticated statistical modeling, econometric analysis, machine learning, time series analysis, and data science tasks through natural conversation. ## 🚀 Quick Start (30 seconds) @@ -83,7 +83,7 @@ Formula building, error recovery, example datasets → *"Help me build a regress ## 📦 Installation ### Prerequisites -- **Python 3.10+** +- **Python 3.11+** - **R 4.4.0+** with **comprehensive package ecosystem**: RMCP uses a systematic 429-package whitelist from CRAN task views organized into 19+ categories: ```r @@ -94,11 +94,11 @@ install.packages(c( )) # Full ecosystem automatically available: Machine Learning (61 packages), -# Econometrics (55 packages), Time Series (48 packages), -# Bayesian Analysis (32 packages), and more +# Econometrics (55 packages), Time Series (57 packages), +# Bayesian Analysis (40 packages), and more ``` -**Package Selection**: Evidence-based using CRAN task views, download statistics, and 4-tier security assessment +**Package Selection**: Evidence-based, using CRAN task views and download statistics ### Install RMCP @@ -106,7 +106,8 @@ install.packages(c( # Standard installation pip install rmcp -# With HTTP transport support +# The Streamable HTTP transport ships in the base install. +# This extra adds pandas/openpyxl for Excel data handling. pip install rmcp[http] # Development installation @@ -215,13 +216,13 @@ docker run -e RMCP_HTTP_HOST=0.0.0.0 -e RMCP_HTTP_PORT=8000 rmcp:latest ## 🔥 Key Features - **🎯 Natural Conversation**: Ask questions in plain English, get statistical analysis -- **📚 Comprehensive Package Ecosystem**: 429 R packages from systematic CRAN task views with 4-tier security system +- **📚 Comprehensive Package Ecosystem**: 429 R packages from systematic CRAN task views - **📊 Professional Output**: Formatted results with markdown tables and inline visualizations - **🔒 Production Ready**: Official MCP SDK with stdio and Streamable HTTP transports, plus bearer-token auth for remote deployments - **⚙️ Flexible Configuration**: Environment variables, config files, and CLI options - **⚡ Fast & Reliable**: 100% test success rate across all scenarios - **🌐 Multiple Transports**: stdio (Claude Desktop) and HTTP (web applications) -- **🛡️ Secure**: Evidence-based package selection with security-conscious permission tiers +- **🛡️ Guardrails**: Package allowlist, explicit user approval for file writes, package installs and system calls, and filesystem confinement for tool-written files. These guard against mistakes, not adversaries — RMCP executes R as the invoking user, so run it as a trusted local tool rather than an untrusted multi-tenant service. ## 📚 Documentation @@ -252,15 +253,13 @@ cd rmcp pip install -e ".[dev]" # Run tests -python tests/unit/test_new_tools.py -python tests/e2e/test_claude_desktop_scenarios.py +uv run pytest tests/ -# Format code -black rmcp/ +# Lint and format +uv run ruff check --fix . +uv run ruff format . ``` -See [CONTRIBUTING.md](CONTRIBUTING.md) for detailed guidelines. - ## 📄 License MIT License - see [LICENSE](LICENSE) file for details. @@ -281,7 +280,8 @@ rmcp check-r-packages # Check what's missing **MCP connection issues?** ```bash -echo '{"jsonrpc":"2.0","id":1,"method":"tools/list","params":{}}' | rmcp start +rmcp list-capabilities # verify tools register without starting a session +rmcp --debug start # run the server with verbose logging on stderr ``` **📖 Need more help?** Check the [examples](examples/) directory for working code. @@ -294,4 +294,3 @@ echo '{"jsonrpc":"2.0","id":1,"method":"tools/list","params":{}}' | rmcp start --- **Ready to turn conversations into statistical insights?** Install RMCP and start analyzing data through AI assistants today! 🚀 -# Test comment diff --git a/docs/installation.md b/docs/installation.md index af6e615..add7412 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -45,7 +45,7 @@ Verify the installation works: # Check version rmcp --version -# List available tools (should show 53 tools) +# List available tools rmcp list-capabilities # Start the server diff --git a/pyproject.toml b/pyproject.toml index a4b6eff..8832db6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,7 +1,7 @@ [project] name = "rmcp" -version = "0.9.1" -description = "Comprehensive Model Context Protocol server with 52 statistical analysis tools for Claude Desktop and Claude web, featuring HTTP transport, interactive documentation, and production deployment" +version = "0.10.0" +description = "Model Context Protocol server bringing R statistical analysis to Claude Desktop and Claude web, over stdio and Streamable HTTP" authors = [{name = "Gaurav Sood", email = "gsood07@gmail.com"}] license = {text = "MIT"} readme = "README.md" @@ -103,6 +103,11 @@ addopts = [ "--strict-markers", "--strict-config", ] +filterwarnings = [ + # A test that returns instead of asserting silently passes no matter what + # it computed. Make that a hard failure rather than a warning. + "error::pytest.PytestReturnNotNoneWarning", +] asyncio_mode = "auto" # asyncio_default_fixture_loop_scope removed for backwards compatibility with older pytest-asyncio versions markers = [ diff --git a/rmcp/bidirectional.py b/rmcp/bidirectional.py index 8915abf..3fe4361 100644 --- a/rmcp/bidirectional.py +++ b/rmcp/bidirectional.py @@ -443,10 +443,7 @@ async def setup_r_bidirectional( ) """ - # Execute setup in R session - setup_result = await context.execute_r_with_session( - setup_script, {}, use_session=True - ) + setup_result = await context.execute_r(setup_script, {}) await context.info("R bidirectional communication set up successfully") diff --git a/rmcp/cli.py b/rmcp/cli.py index 0a45d13..942e996 100644 --- a/rmcp/cli.py +++ b/rmcp/cli.py @@ -6,7 +6,6 @@ import asyncio import json -import logging import os import sys from pathlib import Path @@ -49,7 +48,7 @@ ) @click.pass_context def cli(ctx, config: Path, debug: bool, log_format: str): - """RMCP MCP Server - Comprehensive statistical analysis with 44 tools across 11 categories.""" + """RMCP MCP Server - Comprehensive statistical analysis through R.""" # Ensure context object exists ctx.ensure_object(dict) @@ -188,8 +187,9 @@ def serve( config_file: str | None, ): """Run MCP server with advanced configuration options.""" - # Set logging level - logging.getLogger().setLevel(getattr(logging, log_level)) + # Structlog must be configured before anything logs: its default factory + # writes to stdout, which would corrupt the stdio JSON-RPC stream. + configure_structured_logging(level=log_level, enable_console=True) logger.info(f"Starting RMCP MCP Server v{__version__}") try: # Check R version compatibility @@ -367,6 +367,7 @@ def serve_http( host=effective_host, port=effective_port, api_keys=resolved_keys, + cors_origins=list(config.http.cors_origins), ssl_keyfile=effective_ssl_keyfile, ssl_certfile=effective_ssl_certfile, ssl_keyfile_password=effective_ssl_keyfile_password, @@ -658,7 +659,12 @@ def _register_builtin_tools(server): write_excel, write_json, ) - from .tools.flexible_r import execute_r_analysis, list_allowed_r_packages + from .tools.flexible_r import ( + approve_operation, + approve_r_package, + execute_r_analysis, + list_allowed_r_packages, + ) from .tools.formula_builder import build_formula, validate_formula from .tools.helpers import load_example, suggest_fix, validate_data @@ -746,6 +752,9 @@ def _register_builtin_tools(server): # Flexible R execution execute_r_analysis, list_allowed_r_packages, + # Operation approval -- execute_r_analysis directs the model to these + approve_operation, + approve_r_package, # Advanced MCP Integration - R Session Management list_r_objects, inspect_r_object, @@ -757,9 +766,7 @@ def _register_builtin_tools(server): setup_r_bidirectional, list_callback_sessions, ) - logger.info( - "Registered comprehensive statistical analysis tools (52 total: 44 core + 8 advanced MCP)" - ) + logger.info("Registered statistical analysis tools", count=len(server.tools._tools)) if __name__ == "__main__": diff --git a/rmcp/config/defaults.py b/rmcp/config/defaults.py index d31ecc1..18eb685 100644 --- a/rmcp/config/defaults.py +++ b/rmcp/config/defaults.py @@ -22,6 +22,7 @@ "timeout": 120, "session_timeout": 3600, "max_sessions": 10, + "max_concurrent": 4, "binary_path": None, "version_check_timeout": 30, }, @@ -75,6 +76,7 @@ "RMCP_R_TIMEOUT": "r.timeout", "RMCP_R_SESSION_TIMEOUT": "r.session_timeout", "RMCP_R_MAX_SESSIONS": "r.max_sessions", + "RMCP_R_MAX_CONCURRENT": "r.max_concurrent", "RMCP_R_BINARY_PATH": "r.binary_path", "RMCP_R_VERSION_CHECK_TIMEOUT": "r.version_check_timeout", "RMCP_VFS_MAX_FILE_SIZE": "security.vfs_max_file_size", diff --git a/rmcp/config/loader.py b/rmcp/config/loader.py index 3741cb0..5c8fa9d 100644 --- a/rmcp/config/loader.py +++ b/rmcp/config/loader.py @@ -226,7 +226,14 @@ def _convert_env_value(self, env_var: str, value: str) -> Any: # Boolean conversion return value.lower() in ("true", "1", "yes", "on") case var if var.endswith( - ("_PORT", "_TIMEOUT", "_MAX_SESSIONS", "_MAX_WORKERS", "_MAX_FILE_SIZE") + ( + "_PORT", + "_TIMEOUT", + "_MAX_SESSIONS", + "_MAX_CONCURRENT", + "_MAX_WORKERS", + "_MAX_FILE_SIZE", + ) ): # Integer conversion try: diff --git a/rmcp/config/models.py b/rmcp/config/models.py index be2d5bb..b41ce51 100644 --- a/rmcp/config/models.py +++ b/rmcp/config/models.py @@ -144,6 +144,9 @@ class RConfig: max_sessions: int = 10 """Maximum concurrent R sessions. Limit based on available memory.""" + max_concurrent: int = 4 + """Maximum R subprocesses running at once. Further calls queue.""" + binary_path: str | None = None """Custom R binary path. Auto-detected if None.""" @@ -412,6 +415,11 @@ def _validate(self): f"R max sessions must be positive, got: {self.r.max_sessions}" ) + if self.r.max_concurrent <= 0: + raise ValueError( + f"R max concurrent must be positive, got: {self.r.max_concurrent}" + ) + # Security validation if self.security.vfs_max_file_size <= 0: raise ValueError( diff --git a/rmcp/config/schema.py b/rmcp/config/schema.py index 62affdb..a1e4e1e 100644 --- a/rmcp/config/schema.py +++ b/rmcp/config/schema.py @@ -80,6 +80,12 @@ "minimum": 1, "default": 10, }, + "max_concurrent": { + "type": "integer", + "description": "Maximum R subprocesses running at once", + "minimum": 1, + "default": 4, + }, "binary_path": { "type": ["string", "null"], "description": "Custom R binary path (auto-detect if null)", diff --git a/rmcp/core/context.py b/rmcp/core/context.py index 5b7bf07..13f3f9b 100644 --- a/rmcp/core/context.py +++ b/rmcp/core/context.py @@ -55,10 +55,10 @@ class LifespanState: vfs: Any | None = None # Logging current_log_level: str = "info" - # R Session Management - r_session_enabled: bool = False - r_session_timeout: float = 3600.0 # 1 hour default - default_r_session_id: str | None = None + # Operation approvals granted by the user. Lives here rather than on Context + # because a fresh Context is built per request -- approvals must outlive it. + approved_operations: dict[str, dict[str, Any]] = field(default_factory=dict) + approved_packages: set[str] = field(default_factory=set) @dataclass @@ -153,88 +153,39 @@ def require_path_access(self, path: Path) -> None: f"Allowed roots: {[str(p) for p in self.lifespan.allowed_paths]}" ) + def require_write_path(self, path: str | Path) -> Path | None: + """Require permission to write ``path``, raising if denied. + + Tools that hand a path to R must call this first: the subprocess writes + the bytes, so this is the last point Python controls the destination. + + Fails open when no VFS is configured -- absent a policy object there is + no policy to enforce, which keeps embedders working. Both CLI entry + points call ``MCPServer.configure()``, so servers always have one. + """ + vfs = getattr(self.lifespan, "vfs", None) + if vfs is None: + return None + return vfs.validate_write_path(path) + def get_cache_path(self, key: str) -> Path | None: """Get cache path for key if caching is enabled.""" if self.lifespan.cache_root: return self.lifespan.cache_root / key return None - # R Session Management helpers - def is_r_session_enabled(self) -> bool: - """Check if R session management is enabled.""" - return self.lifespan.r_session_enabled - - def get_r_session_id(self) -> str | None: - """Get the R session ID for this context.""" - # Check for session ID in request metadata first - session_id = self.request.metadata.get("r_session_id") - if session_id: - return session_id - - # Fall back to lifespan default - return self.lifespan.default_r_session_id - - def set_r_session_id(self, session_id: str) -> None: - """Set the R session ID for this context.""" - self.request.metadata["r_session_id"] = session_id - - async def get_or_create_r_session( - self, working_directory: Path | None = None - ) -> str | None: - """Get or create an R session for this context.""" - if not self.is_r_session_enabled(): - return None - - try: - # Import here to avoid circular imports - from ..r_session import get_session_manager - - session_manager = get_session_manager() - session_id = await session_manager.get_or_create_session( - context=self, - session_id=self.get_r_session_id(), - working_directory=working_directory, - ) - - # Update context with session ID - self.set_r_session_id(session_id) - return session_id - - except Exception as e: - await self.warn(f"Failed to get/create R session: {e}") - return None - - async def execute_r_with_session( - self, script: str, args: dict[str, Any], use_session: bool = True - ) -> dict[str, Any]: + async def execute_r(self, script: str, args: dict[str, Any]) -> dict[str, Any]: """ - Execute R script with optional session support. + Execute an R script. Args: script: R script to execute args: Arguments to pass to script - use_session: Whether to use session (if available) or run statelessly Returns: Script execution results """ - # Try session execution first if enabled and requested - if use_session and self.is_r_session_enabled(): - try: - from ..r_session import get_session_manager - - session_id = await self.get_or_create_r_session() - if session_id: - session_manager = get_session_manager() - return await session_manager.execute_in_session( - session_id, script, args, self - ) - except Exception as e: - await self.warn( - f"Session execution failed, falling back to stateless: {e}" - ) - - # Fall back to stateless execution + # Import here to avoid circular imports from ..r_integration import execute_r_script_async return await execute_r_script_async(script, args, self) diff --git a/rmcp/core/server.py b/rmcp/core/server.py index 8e62515..69547ab 100644 --- a/rmcp/core/server.py +++ b/rmcp/core/server.py @@ -64,6 +64,22 @@ ) +#: Sent to the client once, at initialize. Deliberately not a catalog -- the tool +#: list already is one. This carries only what tools/list cannot express. +SERVER_INSTRUCTIONS = """RMCP runs statistical analyses in R, returning both raw values and formatted summaries. + +Two ways in: +- Named tools (linear_model, arima_model, t_test, ...) cover common analyses with validated inputs and schemas. Prefer these. +- execute_r_analysis runs arbitrary R when no named tool fits. The code must assign its output to a variable named `result`. + +Behaviour worth knowing before you call: +- R packages are restricted to an allowlist. Loading anything outside it fails; call list_allowed_r_packages to check, or approve_r_package to permit one. +- File writes, package installs, and system calls from execute_r_analysis are blocked until approved. When a result reports approval_required, call approve_operation and retry. +- Results over 1000 rows or 50KB are returned as a resource_link (rmcp://data/...) instead of inline; read that URI for the payload. +- Plotting tools return base64-encoded PNGs as image content. +""" + + class MCPServer: """ Main MCP server shell that manages lifecycle and registries. @@ -86,59 +102,7 @@ def __init__( self, name: str = "RMCP MCP Server", version: str | None = None, - description: str = """RMCP provides 44 comprehensive statistical analysis tools through R: - -**Regression & Econometrics (8 tools):** -- Linear/logistic regression with diagnostics and residual analysis -- Panel data regression (fixed/random effects) with robust standard errors -- Instrumental variables (2SLS) regression for causal inference -- Vector autoregression (VAR) models for multivariate time series -- Correlation analysis with significance testing and confidence intervals - -**Time Series Analysis (6 tools):** -- ARIMA modeling with automatic order selection and forecasting -- Time series decomposition (trend, seasonal, remainder components) -- Stationarity testing (ADF, KPSS, Phillips-Perron tests) -- Lag/lead variable creation and differencing transformations - -**Statistical Testing (5 tools):** -- T-tests (one-sample, two-sample, paired) with effect sizes -- ANOVA (one-way, two-way) with post-hoc comparisons -- Chi-square tests for independence and goodness-of-fit -- Normality tests (Shapiro-Wilk, Kolmogorov-Smirnov, Anderson-Darling) - -**Data Analysis & Transformation (9 tools):** -- Comprehensive descriptive statistics with distribution analysis -- Outlier detection using multiple methods (IQR, Z-score, Mahalanobis) -- Data standardization (z-score, min-max, robust scaling) -- Winsorization for outlier treatment and data cleaning -- Professional frequency tables with percentages and cumulative statistics - -**Machine Learning (4 tools):** -- K-means clustering with optimal cluster selection and visualization -- Decision trees for classification and regression with pruning -- Random forest models with variable importance and out-of-bag error - -**Professional Visualizations (6 tools):** -- Scatter plots with trend lines, confidence bands, and grouping -- Time series plots for single/multiple variables with forecasting -- Histograms with density overlays and distribution fitting -- Correlation heatmaps with hierarchical clustering -- Box plots for distribution comparison and outlier identification -- Comprehensive residual diagnostic plots (4-panel analysis) - -**File Operations (3 tools):** -- CSV/Excel/JSON import with automatic type detection -- Data filtering, export, and comprehensive dataset information -- Missing value analysis and data quality reporting - -**Advanced Features:** -- Formula builder: Convert natural language to R statistical formulas -- Error recovery: Intelligent error diagnosis with suggested fixes -- Flexible R execution: Custom R code with 80+ whitelisted packages -- Example datasets: Built-in datasets for testing and learning - -All tools provide professionally formatted output with markdown tables, statistical interpretations, and inline visualizations (base64 images). Results include both raw data and formatted summaries using broom/knitr for publication-ready output.""", + description: str = SERVER_INSTRUCTIONS, ): """ Initialize the MCP server instance. @@ -1098,59 +1062,7 @@ async def _handle_notification(self, method: str, params: dict[str, Any]) -> Non def create_server( name: str = "RMCP MCP Server", version: str | None = None, - description: str = """RMCP provides 44 comprehensive statistical analysis tools through R: - -**Regression & Econometrics (8 tools):** -- Linear/logistic regression with diagnostics and residual analysis -- Panel data regression (fixed/random effects) with robust standard errors -- Instrumental variables (2SLS) regression for causal inference -- Vector autoregression (VAR) models for multivariate time series -- Correlation analysis with significance testing and confidence intervals - -**Time Series Analysis (6 tools):** -- ARIMA modeling with automatic order selection and forecasting -- Time series decomposition (trend, seasonal, remainder components) -- Stationarity testing (ADF, KPSS, Phillips-Perron tests) -- Lag/lead variable creation and differencing transformations - -**Statistical Testing (5 tools):** -- T-tests (one-sample, two-sample, paired) with effect sizes -- ANOVA (one-way, two-way) with post-hoc comparisons -- Chi-square tests for independence and goodness-of-fit -- Normality tests (Shapiro-Wilk, Kolmogorov-Smirnov, Anderson-Darling) - -**Data Analysis & Transformation (9 tools):** -- Comprehensive descriptive statistics with distribution analysis -- Outlier detection using multiple methods (IQR, Z-score, Mahalanobis) -- Data standardization (z-score, min-max, robust scaling) -- Winsorization for outlier treatment and data cleaning -- Professional frequency tables with percentages and cumulative statistics - -**Machine Learning (4 tools):** -- K-means clustering with optimal cluster selection and visualization -- Decision trees for classification and regression with pruning -- Random forest models with variable importance and out-of-bag error - -**Professional Visualizations (6 tools):** -- Scatter plots with trend lines, confidence bands, and grouping -- Time series plots for single/multiple variables with forecasting -- Histograms with density overlays and distribution fitting -- Correlation heatmaps with hierarchical clustering -- Box plots for distribution comparison and outlier identification -- Comprehensive residual diagnostic plots (4-panel analysis) - -**File Operations (3 tools):** -- CSV/Excel/JSON import with automatic type detection -- Data filtering, export, and comprehensive dataset information -- Missing value analysis and data quality reporting - -**Advanced Features:** -- Formula builder: Convert natural language to R statistical formulas -- Error recovery: Intelligent error diagnosis with suggested fixes -- Flexible R execution: Custom R code with 80+ whitelisted packages -- Example datasets: Built-in datasets for testing and learning - -All tools provide professionally formatted output with markdown tables, statistical interpretations, and inline visualizations (base64 images). Results include both raw data and formatted summaries using broom/knitr for publication-ready output.""", + description: str = SERVER_INSTRUCTIONS, ) -> MCPServer: """ Factory function to create a new MCP server instance. diff --git a/rmcp/discovery.py b/rmcp/discovery.py deleted file mode 100644 index bde13a6..0000000 --- a/rmcp/discovery.py +++ /dev/null @@ -1,483 +0,0 @@ -""" -Dynamic Tool Discovery for RMCP. - -This module provides dynamic discovery and registration of R statistical analysis tools, -inspired by mcptools' flexible tool composition capabilities. - -Key features: -- Automatic R script discovery and metadata extraction -- Dynamic tool registration at runtime -- Tool composition and dependency management -- Hot-reload capability for development -- Schema validation and compatibility checking - -Design principles: -- Scripts can self-describe their MCP tool interface -- Backward compatibility with existing static tool registration -- Extensible architecture for plugin-style tool addition -- Graceful degradation when tools are unavailable -""" - -import asyncio -import json -import logging -import re -from collections.abc import Callable -from pathlib import Path -from typing import Any - -from .core.context import Context -from .r_integration import execute_r_script_async -from .registries.tools import tool - -logger = logging.getLogger(__name__) - - -class RToolMetadata: - """Metadata for dynamically discovered R tools.""" - - def __init__( - self, - script_path: Path, - name: str, - description: str, - input_schema: dict[str, Any], - output_schema: dict[str, Any] | None = None, - category: str | None = None, - dependencies: list[str] | None = None, - examples: list[dict[str, Any]] | None = None, - ): - self.script_path = script_path - self.name = name - self.description = description - self.input_schema = input_schema - self.output_schema = output_schema - self.category = category - self.dependencies = dependencies or [] - self.examples = examples or [] - self.last_modified = script_path.stat().st_mtime if script_path.exists() else 0 - - -class RToolDiscovery: - """ - Discovers and manages R statistical analysis tools dynamically. - - This class scans directories for R scripts that follow the RMCP tool - convention and can automatically register them as MCP tools. - """ - - def __init__(self, script_directories: list[Path] | None = None): - """ - Initialize tool discovery. - - Args: - script_directories: Directories to scan for R scripts - """ - self.script_directories = script_directories or [] - self.discovered_tools: dict[str, RToolMetadata] = {} - self.registered_tools: set[str] = set() - self._file_mtimes: dict[Path, float] = {} - - def add_script_directory(self, directory: Path) -> None: - """Add a directory to scan for R tools.""" - if directory.exists() and directory.is_dir(): - self.script_directories.append(directory) - logger.info(f"Added R script directory: {directory}") - else: - logger.warning(f"Script directory does not exist: {directory}") - - async def discover_tools( - self, force_refresh: bool = False - ) -> dict[str, RToolMetadata]: - """ - Discover R tools in configured directories. - - Args: - force_refresh: Force rediscovery even if files haven't changed - - Returns: - Dictionary of discovered tool metadata - """ - discovered = {} - - for directory in self.script_directories: - if not directory.exists(): - continue - - # Find all R scripts - r_scripts = list(directory.rglob("*.R")) - - for script_path in r_scripts: - try: - # Check if file has been modified - current_mtime = script_path.stat().st_mtime - if ( - not force_refresh - and script_path in self._file_mtimes - and self._file_mtimes[script_path] == current_mtime - ): - # File hasn't changed, use cached metadata - tool_name = self._get_tool_name_from_path(script_path) - if tool_name in self.discovered_tools: - discovered[tool_name] = self.discovered_tools[tool_name] - continue - - # Extract tool metadata from script - metadata = await self._extract_tool_metadata(script_path) - if metadata: - discovered[metadata.name] = metadata - self._file_mtimes[script_path] = current_mtime - - except Exception as e: - logger.warning(f"Failed to process R script {script_path}: {e}") - - self.discovered_tools.update(discovered) - logger.info(f"Discovered {len(discovered)} R tools") - return discovered - - async def _extract_tool_metadata(self, script_path: Path) -> RToolMetadata | None: - """Extract MCP tool metadata from R script.""" - try: - content = script_path.read_text(encoding="utf-8") - - # Look for RMCP tool metadata in comments - metadata = self._parse_rmcp_metadata(content) - if metadata: - return RToolMetadata( - script_path=script_path, - name=metadata.get( - "name", self._get_tool_name_from_path(script_path) - ), - description=metadata.get( - "description", f"R tool from {script_path.name}" - ), - input_schema=metadata.get( - "input_schema", self._infer_input_schema(content) - ), - output_schema=metadata.get("output_schema"), - category=metadata.get( - "category", self._infer_category_from_path(script_path) - ), - dependencies=metadata.get("dependencies", []), - examples=metadata.get("examples", []), - ) - - # Fallback to convention-based discovery - tool_name = self._get_tool_name_from_path(script_path) - if self._is_valid_rmcp_tool(content): - return RToolMetadata( - script_path=script_path, - name=tool_name, - description=f"Statistical analysis tool: {tool_name}", - input_schema=self._infer_input_schema(content), - category=self._infer_category_from_path(script_path), - ) - - except Exception as e: - logger.warning(f"Error extracting metadata from {script_path}: {e}") - - return None - - def _parse_rmcp_metadata(self, content: str) -> dict[str, Any] | None: - """Parse RMCP tool metadata from R script comments.""" - # Look for RMCP metadata block - metadata_pattern = r"#\s*RMCP-TOOL-START\s*\n(.*?)\n#\s*RMCP-TOOL-END" - match = re.search(metadata_pattern, content, re.DOTALL) - - if match: - try: - metadata_text = match.group(1) - # Remove comment markers and parse as JSON - cleaned_text = re.sub(r"^#\s*", "", metadata_text, flags=re.MULTILINE) - return json.loads(cleaned_text) - except json.JSONDecodeError as e: - logger.warning(f"Invalid JSON in RMCP metadata: {e}") - - return None - - def _infer_input_schema(self, content: str) -> dict[str, Any]: - """Infer input schema from R script content.""" - # Basic schema inference based on common patterns - schema: dict[str, Any] = {"type": "object", "properties": {}} - - # Look for common parameter patterns - if "args$data" in content: - schema["properties"]["data"] = { - "type": "object", - "description": "Input dataset", - } - - if "args$formula" in content: - schema["properties"]["formula"] = { - "type": "string", - "description": "Statistical formula", - } - - if "args$method" in content: - schema["properties"]["method"] = { - "type": "string", - "description": "Analysis method", - } - - # Add basic data property if none found - if not schema["properties"]: - schema["properties"]["data"] = { - "type": "object", - "description": "Input data for analysis", - } - - return schema - - def _infer_category_from_path(self, script_path: Path) -> str: - """Infer tool category from script path.""" - path_parts = script_path.parts - - # Look for category in parent directory names - categories = { - "descriptive": "descriptive_statistics", - "regression": "regression_analysis", - "timeseries": "time_series", - "machine_learning": "machine_learning", - "statistical_tests": "statistical_tests", - "fileops": "file_operations", - "visualization": "data_visualization", - "econometrics": "econometrics", - "transforms": "data_transformation", - "helpers": "helper_tools", - } - - for part in path_parts: - if part in categories: - return categories[part] - - return "statistical_analysis" - - def _get_tool_name_from_path(self, script_path: Path) -> str: - """Generate tool name from script path.""" - # Remove .R extension and convert to snake_case - name = script_path.stem - return re.sub(r"[^a-zA-Z0-9_]", "_", name).lower() - - def _is_valid_rmcp_tool(self, content: str) -> bool: - """Check if R script follows RMCP tool conventions.""" - # Check for required patterns - required_patterns = [ - r"args\s*<-\s*fromJSON", # JSON argument parsing - r"library\(jsonlite\)", # jsonlite usage - r"result\s*<-", # Result assignment - ] - - return all(re.search(pattern, content) for pattern in required_patterns) - - async def create_dynamic_tool(self, metadata: RToolMetadata) -> Callable: - """Create a dynamic MCP tool from R script metadata.""" - - @tool( - name=metadata.name, - description=metadata.description, - input_schema=metadata.input_schema, - ) - async def dynamic_r_tool( - context: Context, params: dict[str, Any] - ) -> dict[str, Any]: - """Dynamically generated R tool.""" - try: - # Check dependencies - if metadata.dependencies: - await self._check_dependencies(metadata.dependencies, context) - - # Execute R script with parameters - script_content = metadata.script_path.read_text(encoding="utf-8") - result = await context.execute_r_with_session(script_content, params) - - # Add metadata to result - result.update( - { - "_tool_metadata": { - "name": metadata.name, - "category": metadata.category, - "script_path": str(metadata.script_path), - "dynamically_loaded": True, - } - } - ) - - return result - - except Exception as e: - await context.error(f"Dynamic tool {metadata.name} failed: {e}") - return {"error": str(e), "tool_name": metadata.name, "success": False} - - # Attach metadata to function - dynamic_r_tool._rmcp_metadata = metadata # type: ignore[attr-defined] - return dynamic_r_tool - - async def _check_dependencies( - self, dependencies: list[str], context: Context - ) -> None: - """Check if R package dependencies are available.""" - for package in dependencies: - check_script = f""" - if (!requireNamespace("{package}", quietly = TRUE)) {{ - stop("Required package '{package}' is not available") - }} - result <- list(package = "{package}", available = TRUE) - """ - - try: - await execute_r_script_async(check_script, {}, context) - except Exception: - raise Exception(f"Missing R package dependency: {package}") - - async def register_discovered_tools(self, tools_registry) -> int: - """Register discovered tools with the MCP tools registry.""" - registered_count = 0 - - for tool_name, metadata in self.discovered_tools.items(): - if tool_name not in self.registered_tools: - try: - # Create dynamic tool function - tool_func = await self.create_dynamic_tool(metadata) - - # Register with tools registry - tools_registry.register_function(tool_func) - - self.registered_tools.add(tool_name) - registered_count += 1 - - logger.info(f"Registered dynamic R tool: {tool_name}") - - except Exception as e: - logger.error(f"Failed to register dynamic tool {tool_name}: {e}") - - return registered_count - - -class RToolComposer: - """ - Composes and manages R tool workflows. - - This class provides capabilities for combining multiple R tools - into complex analytical workflows, similar to mcptools' composition features. - """ - - def __init__(self, discovery: RToolDiscovery): - self.discovery = discovery - self.workflows: dict[str, dict[str, Any]] = {} - - async def create_workflow( - self, name: str, steps: list[dict[str, Any]], description: str | None = None - ) -> None: - """Create a multi-step analytical workflow.""" - workflow = { - "name": name, - "description": description or f"Analytical workflow: {name}", - "steps": steps, - "created_at": asyncio.get_event_loop().time(), - } - - # Validate workflow steps - for i, step in enumerate(steps): - if "tool" not in step: - raise ValueError(f"Step {i} missing 'tool' specification") - - tool_name = step["tool"] - if tool_name not in self.discovery.discovered_tools: - raise ValueError(f"Tool '{tool_name}' not found in discovered tools") - - self.workflows[name] = workflow - logger.info(f"Created workflow: {name} with {len(steps)} steps") - - async def execute_workflow( - self, workflow_name: str, initial_data: dict[str, Any], context: Context - ) -> dict[str, Any]: - """Execute a multi-step workflow.""" - if workflow_name not in self.workflows: - raise ValueError(f"Workflow '{workflow_name}' not found") - - workflow = self.workflows[workflow_name] - steps = workflow["steps"] - - # Track data flow between steps - workflow_data = initial_data.copy() - step_results = [] - - await context.info(f"Starting workflow: {workflow_name}") - - for i, step in enumerate(steps): - tool_name = step["tool"] - step_params = step.get("parameters", {}) - - # Merge workflow data with step parameters - execution_params = {**workflow_data, **step_params} - - await context.progress( - f"Executing step {i + 1}: {tool_name}", i + 1, len(steps) - ) - - try: - # Execute tool step - metadata = self.discovery.discovered_tools[tool_name] - tool_func = await self.discovery.create_dynamic_tool(metadata) - - step_result = await tool_func(context, execution_params) - step_results.append( - {"step": i + 1, "tool": tool_name, "result": step_result} - ) - - # Update workflow data with step results - if isinstance(step_result, dict): - workflow_data.update(step_result) - - except Exception as e: - await context.error(f"Workflow step {i + 1} failed: {e}") - return { - "workflow": workflow_name, - "failed_at_step": i + 1, - "error": str(e), - "completed_steps": step_results, - "success": False, - } - - await context.info(f"Workflow {workflow_name} completed successfully") - - return { - "workflow": workflow_name, - "steps_completed": len(steps), - "results": step_results, - "final_data": workflow_data, - "success": True, - } - - -# Global discovery instance -_tool_discovery: RToolDiscovery | None = None - - -def get_tool_discovery() -> RToolDiscovery: - """Get the global R tool discovery instance.""" - global _tool_discovery - if _tool_discovery is None: - _tool_discovery = RToolDiscovery() - return _tool_discovery - - -async def initialize_tool_discovery( - script_directories: list[Path] | None = None, -) -> None: - """Initialize tool discovery with script directories.""" - discovery = get_tool_discovery() - - if script_directories: - for directory in script_directories: - discovery.add_script_directory(directory) - - # Discover tools - await discovery.discover_tools() - logger.info("R tool discovery initialized") - - -async def register_dynamic_tools(tools_registry) -> int: - """Register all discovered tools with the MCP registry.""" - discovery = get_tool_discovery() - return await discovery.register_discovered_tools(tools_registry) diff --git a/rmcp/r_assets/README.md b/rmcp/r_assets/README.md index e50cf66..099bd2e 100644 --- a/rmcp/r_assets/README.md +++ b/rmcp/r_assets/README.md @@ -12,7 +12,7 @@ rmcp/r_assets/ ├── .lintr # Linting configuration ├── R/ # R package source code │ └── utils.R # Shared utility functions -├── scripts/ # Statistical analysis scripts (44 tools across 11 categories) +├── scripts/ # Statistical analysis scripts, by category │ ├── descriptive/ # Summary statistics, frequency tables, outlier detection │ ├── regression/ # Linear models, correlation, logistic regression │ ├── timeseries/ # ARIMA, decomposition, stationarity tests diff --git a/rmcp/r_assets/scripts/fileops/write_csv.R b/rmcp/r_assets/scripts/fileops/write_csv.R index f59db03..166db79 100644 --- a/rmcp/r_assets/scripts/fileops/write_csv.R +++ b/rmcp/r_assets/scripts/fileops/write_csv.R @@ -2,16 +2,16 @@ # ================================ # # This script writes data to CSV files with formatting options including -# row names, missing value representation, and append mode support. +# row names and missing value representation. # Prepare data and parameters file_path <- args$file_path include_rownames <- args$include_rownames %||% FALSE na_string <- args$na_string %||% "" -append_mode <- args$append %||% FALSE -# Write CSV -write.csv(data, file_path, row.names = include_rownames, na = na_string, append = append_mode) +# Write CSV. Note write.csv always truncates: it refuses `append` outright +# ("attempt to set 'append' ignored"), so appending is not offered here. +write.csv(data, file_path, row.names = include_rownames, na = na_string) # Verify file was written if (!file.exists(file_path)) { diff --git a/rmcp/r_integration.py b/rmcp/r_integration.py index 27ad65d..5f21747 100644 --- a/rmcp/r_integration.py +++ b/rmcp/r_integration.py @@ -27,6 +27,7 @@ import subprocess import tempfile import time +import weakref from pathlib import Path from typing import Any @@ -34,8 +35,24 @@ from .logging_config import get_logger, log_r_execution logger = get_logger(__name__) -# Global semaphore for R process concurrency (max 4 concurrent R processes) -R_SEMAPHORE = asyncio.Semaphore(4) + +# One R-process limiter per event loop. A single module-level Semaphore would +# bind to whichever loop first made a caller wait, then raise "bound to a +# different event loop" in every other loop. Keyed weakly so closed loops go. +_R_SEMAPHORES: weakref.WeakKeyDictionary[ + asyncio.AbstractEventLoop, asyncio.Semaphore +] = weakref.WeakKeyDictionary() + + +def get_r_semaphore() -> asyncio.Semaphore: + """Return the R-process concurrency limiter for the running event loop.""" + loop = asyncio.get_running_loop() + semaphore = _R_SEMAPHORES.get(loop) + if semaphore is None: + semaphore = asyncio.Semaphore(get_config().r.max_concurrent) + _R_SEMAPHORES[loop] = semaphore + return semaphore + # Cached R binary path for performance _R_BINARY_PATH = None @@ -389,7 +406,7 @@ def execute_r_script(script: str, args: dict[str, Any]) -> dict[str, Any]: async def execute_r_script_async( - script: str, args: dict[str, Any], context=None + script: str, args: dict[str, Any], context=None, timeout: float | None = None ) -> dict[str, Any]: """ Execute R script asynchronously with proper cancellation support and concurrency control. @@ -403,13 +420,15 @@ async def execute_r_script_async( script: R script code to execute args: Arguments to pass to the R script as JSON context: Optional context for progress reporting and logging + timeout: Per-call timeout in seconds, overriding the configured default Returns: dict[str, Any]: Result data from R script execution Raises: RExecutionError: If R script execution fails asyncio.CancelledError: If the operation is cancelled """ - async with R_SEMAPHORE: # Limit concurrent R processes + effective_timeout = timeout if timeout is not None else get_config().r.timeout + async with get_r_semaphore(): # Limit concurrent R processes start_time = time.time() # Create temporary files for script, arguments, and results with ( @@ -527,15 +546,15 @@ async def monitor_stderr(): f"Failed to parse progress message: {e}" ) - # Run stdout and stderr monitoring concurrently using TaskGroup (Python 3.11+) - async with asyncio.TaskGroup() as tg: - tg.create_task(read_stdout()) - tg.create_task(monitor_stderr()) - tg.create_task( - asyncio.wait_for( - proc.wait(), timeout=get_config().r.timeout - ) - ) + # The timeout must wrap the whole TaskGroup. Timing out a + # sibling task instead surfaces as a BaseExceptionGroup, which + # the `except TimeoutError` below cannot catch -- leaving the R + # process orphaned. + async with asyncio.timeout(effective_timeout): + async with asyncio.TaskGroup() as tg: + tg.create_task(read_stdout()) + tg.create_task(monitor_stderr()) + tg.create_task(proc.wait()) # Combine output stdout = ( b"".join(stdout_chunks).decode("utf-8") if stdout_chunks else "" @@ -553,11 +572,13 @@ async def monitor_stderr(): await proc.wait() raise except TimeoutError: - logger.error("R script execution timed out") + logger.error( + "R script execution timed out", timeout=effective_timeout + ) proc.kill() await proc.wait() raise RExecutionError( - "R script execution timed out after 120 seconds", + f"R script execution timed out after {effective_timeout} seconds", stdout="", stderr="Execution timed out", returncode=-1, @@ -886,6 +907,7 @@ async def execute_r_script_with_image_async( include_image: bool = True, image_width: int = 800, image_height: int = 600, + timeout: float | None = None, ) -> dict[str, Any]: """ Execute R script asynchronously and optionally include base64-encoded image data. @@ -898,6 +920,7 @@ async def execute_r_script_with_image_async( include_image: Whether to attempt image capture and encoding image_width: Width of captured image in pixels image_height: Height of captured image in pixels + timeout: Per-call timeout in seconds, overriding the configured default Returns: Dict containing R script results, optionally with image_data and image_mime_type """ @@ -914,14 +937,16 @@ async def execute_r_script_with_image_async( } ) # Execute the enhanced script asynchronously - result = await execute_r_script_async(enhanced_script, enhanced_args) + result = await execute_r_script_async( + enhanced_script, enhanced_args, timeout=timeout + ) # Check if the script included image data if isinstance(result, dict) and result.get("image_data"): result["image_mime_type"] = "image/png" return result else: # Standard execution without image support - return await execute_r_script_async(script, args) + return await execute_r_script_async(script, args, timeout=timeout) def diagnose_r_installation() -> dict[str, Any]: diff --git a/rmcp/r_session.py b/rmcp/r_session.py deleted file mode 100644 index 1902747..0000000 --- a/rmcp/r_session.py +++ /dev/null @@ -1,469 +0,0 @@ -""" -R Session Management for RMCP. - -This module provides persistent R session management capabilities inspired by mcptools, -allowing for stateful interactions between the MCP server and R environments. - -Key features: -- Persistent R sessions with workspace state -- Session discovery and lifecycle management -- Environment introspection and object management -- Context-aware tool execution with session persistence -- Graceful session cleanup and resource management - -Design principles: -- Sessions are optional - tools can still run statelessly -- Session state is isolated per context/user -- Automatic cleanup of abandoned sessions -- Transparent fallback to stateless execution -""" - -import asyncio -import json -import logging -import subprocess -import time -import uuid -from dataclasses import dataclass, field -from pathlib import Path -from typing import Any - -from .core.context import Context -from .r_integration import RExecutionError, get_r_binary_path - -logger = logging.getLogger(__name__) - - -@dataclass -class RSessionInfo: - """Information about an R session.""" - - session_id: str - working_directory: Path - created_at: float - last_accessed: float - process_id: int | None = None - workspace_objects: set[str] = field(default_factory=set) - packages_loaded: set[str] = field(default_factory=set) - metadata: dict[str, Any] = field(default_factory=dict) - - def is_active(self) -> bool: - """Check if session is still active.""" - if self.process_id is None: - return False - - try: - # Check if process is still running - import psutil - - process = psutil.Process(self.process_id) - return process.is_running() - except (ImportError, psutil.NoSuchProcess): - # Fallback if psutil not available - try: - import os - - os.kill(self.process_id, 0) - return True - except (OSError, ProcessLookupError): - return False - - def update_access_time(self) -> None: - """Update last accessed timestamp.""" - self.last_accessed = time.time() - - -class RSessionManager: - """ - Manages persistent R sessions for stateful statistical analysis. - - This class provides session lifecycle management, allowing tools to maintain - workspace state across multiple invocations. Sessions are automatically - cleaned up when they become inactive or are explicitly closed. - """ - - def __init__(self, session_timeout: float = 3600.0, max_sessions: int = 10): - """ - Initialize R session manager. - - Args: - session_timeout: Session timeout in seconds (default: 1 hour) - max_sessions: Maximum number of concurrent sessions - """ - self._sessions: dict[str, RSessionInfo] = {} - self._session_processes: dict[str, asyncio.subprocess.Process] = {} - self._session_timeout = session_timeout - self._max_sessions = max_sessions - self._cleanup_task: asyncio.Task | None = None - self._lock = asyncio.Lock() - - async def start_manager(self) -> None: - """Start the session manager and cleanup task.""" - if self._cleanup_task is None: - self._cleanup_task = asyncio.create_task( - self._cleanup_sessions_periodically() - ) - logger.info("R session manager started") - - async def stop_manager(self) -> None: - """Stop the session manager and cleanup all sessions.""" - if self._cleanup_task: - self._cleanup_task.cancel() - try: - await self._cleanup_task - except asyncio.CancelledError: - pass - self._cleanup_task = None - - # Close all active sessions - session_ids = list(self._sessions.keys()) - for session_id in session_ids: - await self.close_session(session_id) - - logger.info("R session manager stopped") - - async def get_or_create_session( - self, - context: Context, - session_id: str | None = None, - working_directory: Path | None = None, - ) -> str: - """ - Get existing session or create a new one. - - Args: - context: Current request context - session_id: Specific session ID to get/create - working_directory: Working directory for the session - - Returns: - Session ID - """ - async with self._lock: - # Generate session ID if not provided - if session_id is None: - session_id = f"r_session_{uuid.uuid4().hex[:8]}" - - # Check if session exists and is active - if session_id in self._sessions: - session_info = self._sessions[session_id] - if session_info.is_active(): - session_info.update_access_time() - await context.info(f"Using existing R session: {session_id}") - return session_id - else: - # Session exists but is inactive, remove it - await self._remove_session(session_id) - - # Create new session - return await self._create_session(context, session_id, working_directory) - - async def _create_session( - self, - context: Context, - session_id: str, - working_directory: Path | None = None, - ) -> str: - """Create a new R session.""" - # Check session limits - if len(self._sessions) >= self._max_sessions: - await self._cleanup_oldest_session() - - # Determine working directory - if working_directory is None: - working_directory = Path.cwd() - - # Create session info - session_info = RSessionInfo( - session_id=session_id, - working_directory=working_directory, - created_at=time.time(), - last_accessed=time.time(), - ) - - try: - # Start R process - r_binary = get_r_binary_path() - process = await asyncio.create_subprocess_exec( - r_binary, - "--slave", - "--no-restore", - "--no-save", - stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - cwd=working_directory, - ) - - session_info.process_id = process.pid - self._sessions[session_id] = session_info - self._session_processes[session_id] = process - - # Initialize session with basic setup - await self._initialize_session(session_id, context) - - await context.info( - f"Created new R session: {session_id} (PID: {process.pid})" - ) - return session_id - - except Exception as e: - await context.error(f"Failed to create R session {session_id}: {e}") - if session_id in self._sessions: - del self._sessions[session_id] - raise RExecutionError(f"Failed to create R session: {e}") - - async def _initialize_session(self, session_id: str, context: Context) -> None: - """Initialize a new R session with basic packages and utilities.""" - init_script = f""" - # Load essential packages - if (!require(jsonlite, quietly = TRUE)) {{ - install.packages("jsonlite", repos = "https://cran.r-project.org") - library(jsonlite) - }} - - # Set up session metadata - .rmcp_session_id <- "{session_id}" - .rmcp_session_start <- Sys.time() - - # Define helper functions for session management - .rmcp_list_objects <- function() {{ - objects_info <- ls.str(envir = .GlobalEnv, max.level = 1) - return(ls(envir = .GlobalEnv)) - }} - - .rmcp_get_object_info <- function(name) {{ - if (!exists(name, envir = .GlobalEnv)) {{ - return(list(exists = FALSE)) - }} - obj <- get(name, envir = .GlobalEnv) - list( - exists = TRUE, - class = class(obj), - type = typeof(obj), - length = length(obj), - size = object.size(obj), - summary = capture.output(str(obj)) - ) - }} - - # Session ready - cat("RMCP session initialized\\n") - """ - - try: - await self._execute_in_session(session_id, init_script, context) - except Exception as e: - await context.warn(f"Session initialization warning: {e}") - - async def execute_in_session( - self, session_id: str, script: str, args: dict[str, Any], context: Context - ) -> dict[str, Any]: - """ - Execute R script in specific session. - - Args: - session_id: Target session ID - script: R script to execute - args: Arguments to pass to script - context: Request context - - Returns: - Script execution results - """ - async with self._lock: - if session_id not in self._sessions: - raise RExecutionError(f"Session {session_id} not found") - - session_info = self._sessions[session_id] - if not session_info.is_active(): - await self._remove_session(session_id) - raise RExecutionError(f"Session {session_id} is no longer active") - - session_info.update_access_time() - - # Create execution script with args injection - execution_script = f""" - # Inject arguments - args <- {json.dumps(args)} - - # Execute user script - tryCatch({{ - {script} - - # Return result - if (exists("result")) {{ - cat(toJSON(result, auto_unbox = TRUE)) - }} else {{ - cat(toJSON(list(success = TRUE, message = "Script executed but no result variable set"))) - }} - }}, error = function(e) {{ - cat(toJSON(list(error = TRUE, message = e$message))) - }}) - """ - - return await self._execute_in_session(session_id, execution_script, context) - - async def _execute_in_session( - self, session_id: str, script: str, context: Context - ) -> dict[str, Any]: - """Execute script in session and return results.""" - process = self._session_processes.get(session_id) - if not process: - raise RExecutionError(f"No active process for session {session_id}") - - try: - # Send script to R session - if process.stdin is None: - raise RExecutionError(f"No stdin available for session {session_id}") - script_bytes = (script + "\n").encode("utf-8") - process.stdin.write(script_bytes) - await process.stdin.drain() - - # Read response (this is a simplified version - production would need more robust parsing) - # For now, we'll use the stateless execution as fallback - from .r_integration import execute_r_script_async - - return await execute_r_script_async(script, {}, context) - - except Exception as e: - await context.error(f"Error executing in session {session_id}: {e}") - # Mark session as inactive - await self._remove_session(session_id) - raise RExecutionError(f"Session execution failed: {e}") - - async def list_sessions(self) -> list[dict[str, Any]]: - """List all active sessions with their metadata.""" - sessions = [] - for session_id, session_info in self._sessions.items(): - if session_info.is_active(): - sessions.append( - { - "session_id": session_id, - "working_directory": str(session_info.working_directory), - "created_at": session_info.created_at, - "last_accessed": session_info.last_accessed, - "process_id": session_info.process_id, - "workspace_objects": list(session_info.workspace_objects), - "packages_loaded": list(session_info.packages_loaded), - "metadata": session_info.metadata, - } - ) - return sessions - - async def get_session_info(self, session_id: str) -> dict[str, Any] | None: - """Get detailed information about a specific session.""" - if session_id not in self._sessions: - return None - - session_info = self._sessions[session_id] - if not session_info.is_active(): - await self._remove_session(session_id) - return None - - return { - "session_id": session_id, - "working_directory": str(session_info.working_directory), - "created_at": session_info.created_at, - "last_accessed": session_info.last_accessed, - "process_id": session_info.process_id, - "workspace_objects": list(session_info.workspace_objects), - "packages_loaded": list(session_info.packages_loaded), - "metadata": session_info.metadata, - } - - async def close_session(self, session_id: str) -> bool: - """Close and cleanup a specific session.""" - if session_id not in self._sessions: - return False - - await self._remove_session(session_id) - return True - - async def _remove_session(self, session_id: str) -> None: - """Remove session and cleanup resources.""" - # Close R process - if session_id in self._session_processes: - process = self._session_processes[session_id] - try: - process.terminate() - try: - await asyncio.wait_for(process.wait(), timeout=5.0) - except TimeoutError: - process.kill() - await process.wait() - except Exception as e: - logger.warning(f"Error closing R process for session {session_id}: {e}") - - del self._session_processes[session_id] - - # Remove session info - if session_id in self._sessions: - del self._sessions[session_id] - - logger.info(f"Removed R session: {session_id}") - - async def _cleanup_sessions_periodically(self) -> None: - """Periodically cleanup inactive and expired sessions.""" - while True: - try: - await asyncio.sleep(300) # Check every 5 minutes - await self._cleanup_expired_sessions() - except asyncio.CancelledError: - break - except Exception as e: - logger.error(f"Error in session cleanup: {e}") - - async def _cleanup_expired_sessions(self) -> None: - """Remove expired or inactive sessions.""" - current_time = time.time() - expired_sessions = [] - - for session_id, session_info in self._sessions.items(): - if ( - not session_info.is_active() - or current_time - session_info.last_accessed > self._session_timeout - ): - expired_sessions.append(session_id) - - for session_id in expired_sessions: - await self._remove_session(session_id) - logger.info(f"Cleaned up expired session: {session_id}") - - async def _cleanup_oldest_session(self) -> None: - """Remove the oldest session to make room for a new one.""" - if not self._sessions: - return - - oldest_session_id = min( - self._sessions.keys(), key=lambda sid: self._sessions[sid].last_accessed - ) - - await self._remove_session(oldest_session_id) - logger.info(f"Removed oldest session to make room: {oldest_session_id}") - - -# Global session manager instance -_session_manager: RSessionManager | None = None - - -def get_session_manager() -> RSessionManager: - """Get the global R session manager instance.""" - global _session_manager - if _session_manager is None: - _session_manager = RSessionManager() - return _session_manager - - -async def initialize_session_manager() -> None: - """Initialize the global session manager.""" - manager = get_session_manager() - await manager.start_manager() - - -async def cleanup_session_manager() -> None: - """Cleanup the global session manager.""" - global _session_manager - if _session_manager: - await _session_manager.stop_manager() - _session_manager = None diff --git a/rmcp/registries/resources.py b/rmcp/registries/resources.py index 203bbae..7b40dd0 100644 --- a/rmcp/registries/resources.py +++ b/rmcp/registries/resources.py @@ -23,6 +23,7 @@ from ..r_integration import execute_r_script_async from ..security import VFSError from ..tools.helpers import load_example +from .tools import MAX_STORED_RESULTS logger = logging.getLogger(__name__) @@ -370,7 +371,14 @@ async def _read_stored_rmcp_data( raise ValueError(f"RMCP resource not found: {resource_id}") data_store = tools_registry._large_data_store if resource_id not in data_store: - raise ValueError(f"RMCP resource not found: {resource_id}") + # The store keeps only the most recent results, so an old link is + # expired rather than invalid. Say which, so the caller knows to + # re-run the tool instead of hunting for a bad URI. + raise ValueError( + f"RMCP resource {resource_id} is no longer available. Only the " + f"{MAX_STORED_RESULTS} most recent large results are retained; " + "re-run the tool to regenerate it." + ) stored_resource = data_store[resource_id] data = stored_resource["data"] content_type = stored_resource.get("content_type", "application/json") diff --git a/rmcp/registries/tools.py b/rmcp/registries/tools.py index 30fe032..c007748 100644 --- a/rmcp/registries/tools.py +++ b/rmcp/registries/tools.py @@ -12,6 +12,7 @@ import json import logging import uuid +from collections import OrderedDict from collections.abc import Awaitable, Callable, Sequence from dataclasses import dataclass from typing import Any, Protocol @@ -79,6 +80,11 @@ class ToolDefinition: annotations: dict[str, Any] | None = None +#: How many oversized tool results to keep resolvable via ``rmcp://data/{id}``. +#: Each is at least 50KB, so this store has to be bounded. +MAX_STORED_RESULTS = 32 + + class ToolsRegistry: """Registry for MCP tools with schema validation.""" @@ -88,6 +94,7 @@ def __init__( ): self._tools: dict[str, ToolDefinition] = {} self._on_list_changed = on_list_changed + self._large_data_store: OrderedDict[str, dict[str, Any]] = OrderedDict() def register( self, @@ -125,14 +132,16 @@ async def list_tools( page, next_cursor = _paginate_items(ordered_tools, cursor, limit) tools: list[dict[str, Any]] = [] for tool_def in page: + # outputSchema is deliberately not advertised. It accounted for + # over half the tools/list payload while telling the model only what + # the first result already shows. call_tool still validates every + # result against it, so the shape guarantee is unchanged. tool_info = { "name": tool_def.name, "title": tool_def.title, "description": tool_def.description, "inputSchema": tool_def.input_schema, } - if tool_def.output_schema: - tool_info["outputSchema"] = tool_def.output_schema if tool_def.annotations: tool_info["annotations"] = tool_def.annotations tools.append(tool_info) @@ -394,16 +403,15 @@ def _check_for_large_data_and_create_resource(self, data: Any) -> str | None: if size_bytes > MAX_SIZE_BYTES or is_large_table: resource_id = str(uuid.uuid4()) resource_uri = f"rmcp://data/{resource_id}" - # Store data in server's resource registry - # This is a simplified implementation - in production you might want - # to store in a proper cache/database - if not hasattr(self, "_large_data_store"): - self._large_data_store = {} + # Bounded, oldest-first. Each entry is by definition large, so an + # unbounded store grows without limit in a long-lived server. self._large_data_store[resource_id] = { "data": data, "content_type": "application/json", "size_bytes": size_bytes, } + while len(self._large_data_store) > MAX_STORED_RESULTS: + self._large_data_store.popitem(last=False) return resource_uri except Exception: # If we can't serialize or analyze the data, just return None diff --git a/rmcp/security/vfs.py b/rmcp/security/vfs.py index 32abb61..db98d70 100644 --- a/rmcp/security/vfs.py +++ b/rmcp/security/vfs.py @@ -49,6 +49,9 @@ def __init__( self.read_only = ( read_only if read_only is not None else config.security.vfs_read_only ) + # Directories the user has explicitly approved for writing. These widen + # write access within the sandbox without lifting read_only globally. + self.writable_roots: set[Path] = set() self.max_file_size = ( max_file_size if max_file_size is not None @@ -78,9 +81,41 @@ def __init__( ) logger.info( f"VFS initialized: {len(self.allowed_roots)} roots, " - f"read_only={read_only}, max_size={max_file_size}" + f"read_only={self.read_only}, max_size={self.max_file_size}" ) + def grant_write(self, directory: str | Path) -> Path: + """Approve writes under ``directory`` without lifting read-only globally. + + The directory must already resolve inside an allowed root, so a grant + can widen write access within the sandbox but never escape it. + """ + resolved = self._resolve_and_validate_path(directory) + self.writable_roots.add(resolved) + logger.info(f"VFS write access granted: {resolved}") + return resolved + + def validate_write_path(self, path: str | Path) -> Path: + """Resolve and authorise a write target without requiring it to exist. + + Tools that delegate the actual write to R must call this *before* + handing the path over: once the subprocess starts, Python has no say in + where it writes. + """ + resolved = self._resolve_and_validate_path(path) + if not self._is_writable(resolved): + raise VFSError( + f"Write access denied: {resolved}. The VFS is read-only; " + "approve the directory with approve_operation to enable writing." + ) + return resolved + + def _is_writable(self, path: Path) -> bool: + """Whether ``path`` may be written, honouring per-directory grants.""" + if not self.read_only: + return True + return any(path == root or root in path.parents for root in self.writable_roots) + def _resolve_and_validate_path(self, path: str | Path) -> Path: """Resolve path and validate against allowed roots.""" try: @@ -191,9 +226,9 @@ def list_directory(self, path: str | Path) -> list[dict[str, Any]]: def write_file(self, path: str | Path, content: bytes) -> None: """Write file with security checks.""" - if self.read_only: - raise VFSError("VFS is configured as read-only") resolved_path = self._resolve_and_validate_path(path) + if not self._is_writable(resolved_path): + raise VFSError("VFS is configured as read-only") # Check content size if len(content) > self.max_file_size: raise VFSError( @@ -212,9 +247,9 @@ async def write_file_async(self, path: str | Path, content: bytes) -> None: """Write file asynchronously with security checks.""" import asyncio - if self.read_only: - raise VFSError("VFS is configured as read-only") resolved_path = self._resolve_and_validate_path(path) + if not self._is_writable(resolved_path): + raise VFSError("VFS is configured as read-only") # Check content size if len(content) > self.max_file_size: raise VFSError( @@ -261,7 +296,8 @@ def file_info(self, path: str | Path) -> dict[str, Any]: "mime_type": mime_type, "encoding": encoding, "readable": os.access(resolved_path, os.R_OK), - "writable": os.access(resolved_path, os.W_OK) and not self.read_only, + "writable": os.access(resolved_path, os.W_OK) + and self._is_writable(resolved_path), } except OSError as e: raise VFSError(f"Failed to get file info for {resolved_path}: {e}") diff --git a/rmcp/tools/descriptive.py b/rmcp/tools/descriptive.py index a490928..70b68f6 100644 --- a/rmcp/tools/descriptive.py +++ b/rmcp/tools/descriptive.py @@ -75,7 +75,7 @@ "required": ["statistics", "variables", "n_obs", "grouped"], "additionalProperties": False, }, - description="Computes comprehensive descriptive statistics including mean, median, standard deviation, quantiles, skewness, kurtosis, and distribution shape measures. Supports grouping by categorical variables for comparative analysis. Provides missing value counts and handles different data types appropriately. Use for initial data exploration, understanding variable distributions, identifying data quality issues, or generating summary reports for research and business analytics.", + description="Descriptive statistics: mean, median, SD, quantiles, skewness, kurtosis, and missing counts, optionally by group.", ) async def summary_stats(context, params) -> dict[str, Any]: """Compute comprehensive descriptive statistics.""" @@ -163,7 +163,7 @@ async def summary_stats(context, params) -> dict[str, Any]: ], "additionalProperties": False, }, - description="Identifies outliers using multiple detection methods: Interquartile Range (IQR) method, Z-score (standard deviations from mean), Modified Z-score (using median absolute deviation), or Mahalanobis distance for multivariate outliers. Returns outlier flags, detection thresholds, and visualization-ready results. Use for data cleaning, quality assessment, fraud detection, or preparing data for modeling by identifying unusual observations.", + description="Flags outliers by IQR, Z-score, modified Z-score, or Mahalanobis distance for multivariate data.", ) async def outlier_detection(context, params) -> dict[str, Any]: """Detect outliers in data.""" @@ -253,7 +253,7 @@ async def outlier_detection(context, params) -> dict[str, Any]: "required": ["frequency_tables", "variables", "total_observations"], "additionalProperties": False, }, - description="Creates frequency tables for categorical variables showing counts, percentages, cumulative frequencies, and missing value summaries. Supports multiple variables simultaneously and optional sorting by frequency or value. Provides chi-square goodness-of-fit tests when expected frequencies are specified. Use for categorical data exploration, survey analysis, market research, or understanding distribution of discrete variables.", + description="Counts, percentages, and cumulative frequencies for categorical variables.", ) async def frequency_table(context, params) -> dict[str, Any]: """Generate frequency tables.""" diff --git a/rmcp/tools/econometrics.py b/rmcp/tools/econometrics.py index d04cafb..18f2e58 100644 --- a/rmcp/tools/econometrics.py +++ b/rmcp/tools/econometrics.py @@ -115,7 +115,7 @@ ], "additionalProperties": False, }, - description="Performs panel data regression analysis with fixed effects, random effects, or between/pooling estimators for longitudinal data. Handles unbalanced panels, robust standard errors, and provides Hausman tests for model selection. Essential for analyzing repeated observations on same units over time. Use for causal inference, policy evaluation, individual heterogeneity control, or any analysis with cross-sectional time series data.", + description="Panel regression with fixed, random, between, or pooling estimators. Handles unbalanced panels and reports a Hausman test.", ) async def panel_regression(context, params) -> dict[str, Any]: """Perform panel data regression.""" @@ -253,7 +253,7 @@ async def panel_regression(context, params) -> dict[str, Any]: ], "additionalProperties": False, }, - description="Performs Two-Stage Least Squares (2SLS) instrumental variables regression to address endogeneity bias when explanatory variables are correlated with error terms. Provides first-stage statistics, weak instrument tests, and overidentification tests. Critical for causal inference when randomized experiments are not possible. Use for addressing simultaneity, measurement error, omitted variable bias, or establishing causal relationships in observational data.", + description="Two-stage least squares (2SLS) for endogenous regressors. Returns first-stage statistics, weak-instrument and overidentification tests.", ) async def instrumental_variables(context, params) -> dict[str, Any]: """Perform instrumental variables regression.""" @@ -377,7 +377,7 @@ async def instrumental_variables(context, params) -> dict[str, Any]: ], "additionalProperties": False, }, - description="Estimates Vector Autoregression (VAR) models for analyzing dynamic relationships among multiple time series variables. Each variable is modeled as linear function of its own lags and lags of all other variables. Provides impulse response functions, variance decomposition, and Granger causality tests. Use for macroeconomic modeling, forecasting multiple related time series, understanding dynamic interdependencies, or analyzing shock transmission between variables.", + description="Vector autoregression across multiple time series. Returns impulse response functions, variance decomposition, and Granger causality tests.", ) async def var_model(context, params) -> dict[str, Any]: """Fit Vector Autoregression model.""" diff --git a/rmcp/tools/fileops.py b/rmcp/tools/fileops.py index f7d65a2..28787ed 100644 --- a/rmcp/tools/fileops.py +++ b/rmcp/tools/fileops.py @@ -93,7 +93,7 @@ "required": ["data", "file_info", "parsing_info", "summary"], "additionalProperties": False, }, - description="Reads CSV (Comma-Separated Values) files with flexible parsing options including custom separators, header handling, missing value specifications, and row/column selection. Automatically detects data types and handles various CSV formats. Use for importing datasets, loading experimental data, processing survey results, or reading any tabular data stored in CSV format. Essential first step in most data analysis workflows.", + description="Reads a CSV file, with options for separator, header, missing-value strings, and row/column selection.", ) async def read_csv(context, params) -> dict[str, Any]: """Read CSV file and return data.""" @@ -123,7 +123,6 @@ async def read_csv(context, params) -> dict[str, Any]: "file_path": {"type": "string"}, "include_rownames": {"type": "boolean", "default": False}, "na_string": {"type": "string", "default": ""}, - "append": {"type": "boolean", "default": False}, }, "required": ["data", "file_path"], }, @@ -169,11 +168,14 @@ async def read_csv(context, params) -> dict[str, Any]: ], "additionalProperties": False, }, - description="Writes data to CSV files with customizable formatting options including separators, missing value representations, decimal precision, and column selection. Preserves data types and handles special characters appropriately. Use for exporting analysis results, sharing datasets, creating backups, or preparing data for other applications. Standard format for data interchange and archival.", + description="Writes a dataset to a CSV file.", ) async def write_csv(context, params) -> dict[str, Any]: """Write data to CSV file.""" await context.info("Writing CSV file", file_path=params.get("file_path")) + # Confine the destination before R gets the path -- once the + # subprocess runs, Python no longer controls where it writes. + context.require_write_path(params["file_path"]) # Load R script from separated file r_script = get_r_script("fileops", "write_csv") @@ -245,11 +247,14 @@ async def write_csv(context, params) -> dict[str, Any]: ], "additionalProperties": False, }, - description="Writes data to Excel files (.xlsx) with advanced formatting including multiple worksheets, custom sheet names, cell formatting, and metadata. Preserves data types and provides professional presentation options. Use for business reports, stakeholder presentations, multi-table datasets, or when advanced formatting and multiple sheets are required for data delivery.", + description="Writes one or more datasets to an .xlsx file, one sheet per table.", ) async def write_excel(context, params) -> dict[str, Any]: """Write data to Excel file.""" await context.info("Writing Excel file", file_path=params.get("file_path")) + # Confine the destination before R gets the path -- once the + # subprocess runs, Python no longer controls where it writes. + context.require_write_path(params["file_path"]) # Load R script from separated file r_script = get_r_script("fileops", "write_excel") @@ -343,7 +348,7 @@ async def write_excel(context, params) -> dict[str, Any]: ], "additionalProperties": False, }, - description="Provides comprehensive dataset information including dimensions, column names, data types, missing value counts, memory usage, and basic statistics summary. Essential for initial data exploration and quality assessment. Use to understand dataset structure, identify data quality issues, check for missing values, or generate data documentation and metadata reports.", + description="Dataset dimensions, column names, types, missing-value counts, and memory usage. Use to inspect structure before analysis.", ) async def data_info(context, params) -> dict[str, Any]: """Get comprehensive dataset information.""" @@ -432,7 +437,7 @@ async def data_info(context, params) -> dict[str, Any]: ], "additionalProperties": False, }, - description="Filters datasets using multiple logical conditions with support for numeric comparisons, string matching, date ranges, and complex boolean logic. Supports AND/OR operations and missing value handling. Use for data subsetting, creating analysis samples, removing outliers, selecting specific time periods, or preparing data for focused analysis on particular subgroups.", + description="Subsets rows by one or more conditions, with AND/OR logic and missing-value handling.", ) async def filter_data(context, params) -> dict[str, Any]: """Filter data based on conditions.""" @@ -523,7 +528,7 @@ async def filter_data(context, params) -> dict[str, Any]: "required": ["data", "file_info", "summary"], "additionalProperties": False, }, - description="Reads Excel files (.xlsx, .xls) with flexible options for sheet selection, cell ranges, header detection, and data type specification. Handles multiple worksheets and complex Excel formatting. Use for importing business data, reading formatted reports, processing multi-sheet workbooks, or accessing data stored in Excel's native format with preserving original structure.", + description="Reads an .xlsx or .xls file, with sheet and cell-range selection.", ) async def read_excel(context, params) -> dict[str, Any]: """Read Excel file and return data.""" @@ -617,7 +622,7 @@ async def read_excel(context, params) -> dict[str, Any]: "required": ["data", "file_info", "summary"], "additionalProperties": False, }, - description="Reads JSON files and intelligently converts nested structures to tabular format suitable for statistical analysis. Handles nested objects, arrays, and mixed data types with flexible flattening options. Use for importing API responses, web scraping results, NoSQL database exports, or any hierarchical data that needs conversion to rectangular format for analysis.", + description="Reads a JSON file and flattens nested structures into tabular form.", ) async def read_json(context, params) -> dict[str, Any]: """Read JSON file and return data.""" @@ -703,11 +708,14 @@ async def read_json(context, params) -> dict[str, Any]: ], "additionalProperties": False, }, - description="Writes data to JSON files using column-wise format optimized for statistical software with customizable formatting and compression options. Maintains data type information and handles missing values appropriately. Use for creating web API responses, exporting data for JavaScript applications, archiving datasets, or preparing data for JSON-based analytics platforms.", + description="Writes a dataset to a JSON file in column-wise format.", ) async def write_json(context, params) -> dict[str, Any]: """Write data to JSON file.""" await context.info("Writing JSON file", file_path=params.get("file_path")) + # Confine the destination before R gets the path -- once the + # subprocess runs, Python no longer controls where it writes. + context.require_write_path(params["file_path"]) # Load R script from separated file r_script = get_r_script("fileops", "write_json") diff --git a/rmcp/tools/flexible_r.py b/rmcp/tools/flexible_r.py index 856da01..15a798b 100644 --- a/rmcp/tools/flexible_r.py +++ b/rmcp/tools/flexible_r.py @@ -30,55 +30,9 @@ get_package_categories, ) -# Use comprehensive whitelist (700+ packages from CRAN task views) +# Comprehensive whitelist drawn from CRAN task views ALLOWED_R_PACKAGES = COMPREHENSIVE_PACKAGES -# Legacy small whitelist for conservative environments (if needed) -LEGACY_ALLOWED_PACKAGES = { - # Base R packages (always available) - "base", - "stats", - "graphics", - "grDevices", - "utils", - "datasets", - "methods", - "grid", - "splines", - "stats4", - # Core tidyverse - "dplyr", - "tidyr", - "ggplot2", - "readr", - "tibble", - "stringr", - "forcats", - "lubridate", - "purrr", - "tidyverse", - # Essential statistical packages - "lmtest", - "sandwich", - "car", - "MASS", - "boot", - "survival", - "caret", - "randomForest", - "rpart", - "e1071", - "forecast", - "zoo", - "xts", - # Essential utilities - "jsonlite", - "broom", - "knitr", - "rlang", - "haven", -} - # Dangerous patterns to block # Patterns moved to OPERATION_CATEGORIES for user approval: # - install.packages (now in package_installation) @@ -101,24 +55,74 @@ ] +def _approved_operations(context) -> dict[str, dict[str, Any]]: + """Live approval map for this session. + + Approvals live on the lifespan state, not the Context: a fresh Context is + built for every request, so anything stored there is forgotten immediately. + Returns an empty mapping when no lifespan is available. + """ + lifespan = getattr(context, "lifespan", None) + if lifespan is None: + return {} + return getattr(lifespan, "approved_operations", {}) + + +def _approved_packages(context) -> set[str]: + """Live set of session-approved packages. See :func:`_approved_operations`.""" + lifespan = getattr(context, "lifespan", None) + if lifespan is None: + return set() + return getattr(lifespan, "approved_packages", set()) + + def is_operation_approved( context, operation_type: str, specific_operation: str ) -> bool: """Check if a specific operation has been approved by the user.""" - if not context or not hasattr(context, "_approved_operations"): - return False + return specific_operation in _approved_operations(context).get(operation_type, {}) + + +#: R package names are bare identifiers. A `packages` entry that is anything +#: else is an attempt to break out of the generated ``library(...)`` call and +#: run code that never passes the checks below. +_R_PACKAGE_NAME = re.compile(r"^[A-Za-z][A-Za-z0-9._]*$") - operations = context._approved_operations.get(operation_type, {}) - return specific_operation in operations +def _package_error(pkg: str, context) -> str | None: + """Return an error for a package that may not be loaded, else None.""" + if pkg in ALLOWED_R_PACKAGES or pkg in _approved_packages(context): + return None + if context: + return f"APPROVAL_NEEDED:{pkg}" + return f"Package '{pkg}' requires user approval" -def validate_r_code(r_code: str, context=None) -> tuple[bool, str | None]: + +def validate_r_code( + r_code: str, context=None, packages: list[str] | None = None +) -> tuple[bool, str | None]: """ Validate R code for safety with interactive operation and package approval. + Args: + r_code: the R source to be executed + context: request context, used to look up session approvals + packages: package names the caller asked to load. These are + interpolated into ``library(...)`` lines, so they are validated + here rather than being trusted. + Returns: (is_safe, error_message) """ + # Declared packages first: these bypass every check below if left unvalidated, + # because they are concatenated into the script rather than parsed out of it. + for pkg in packages or []: + if not _R_PACKAGE_NAME.match(pkg): + return False, f"Invalid package name: {pkg!r}" + error = _package_error(pkg, context) + if error: + return False, error + # Check for controllable operations that need approval for operation_type, config in OPERATION_CATEGORIES.items(): for pattern in config["patterns"]: @@ -161,43 +165,15 @@ def validate_r_code(r_code: str, context=None) -> tuple[bool, str | None]: # Extract library/require calls lib_pattern = r"(?:library|require)\s*\(\s*['\"]?(\w+)['\"]?\s*\)" - packages = re.findall(lib_pattern, code_without_comments, re.IGNORECASE) + code_packages = re.findall(lib_pattern, code_without_comments, re.IGNORECASE) - # Check all packages are in whitelist or session-approved - for pkg in packages: - if pkg not in ALLOWED_R_PACKAGES: - # Check if package is session-approved - session_approved = ( - context - and hasattr(context, "_approved_packages") - and pkg in context._approved_packages - ) - if not session_approved: - # Request user approval through context - if context: - return False, f"APPROVAL_NEEDED:{pkg}" - else: - return False, f"Package '{pkg}' requires user approval" - - # Check for double-colon package usage (pkg::function) - colon_pattern = r"(\w+)::" - colon_packages = re.findall(colon_pattern, code_without_comments) - for pkg in colon_packages: - if pkg not in ALLOWED_R_PACKAGES: - # Check if package is session-approved - session_approved = ( - context - and hasattr(context, "_approved_packages") - and pkg in context._approved_packages - ) - if not session_approved: - if context: - return False, f"APPROVAL_NEEDED:{pkg}" - else: - return ( - False, - f"Package '{pkg}' (used with ::) requires user approval", - ) + # ...and double-colon usage (pkg::function) + code_packages += re.findall(r"(\w+)::", code_without_comments) + + for pkg in code_packages: + error = _package_error(pkg, context) + if error: + return False, error return True, None @@ -278,7 +254,7 @@ def validate_r_code(r_code: str, context=None) -> tuple[bool, str | None]: }, "required": ["success"], }, - description="Executes custom R code for advanced statistical analyses beyond the built-in tools, with comprehensive safety validation including package whitelisting, timeout protection, and audit logging. Supports complex statistical procedures, custom visualizations, and specialized analyses not covered by structured tools. Use for cutting-edge statistical methods, custom modeling approaches, research-specific analyses, or when existing tools don't meet specific analytical requirements. Essential for advanced users needing R's full statistical capabilities.", + description="Runs arbitrary R code for analyses the named tools do not cover. Code is checked against the package allowlist; file writes, package installs, and system calls require approval first. Use when no structured tool fits.", ) async def execute_r_analysis(context, params) -> dict[str, Any]: """Execute flexible R code with safety checks.""" @@ -292,10 +268,8 @@ async def execute_r_analysis(context, params) -> dict[str, Any]: await context.info(f"Executing R analysis: {description}") - # Package validation is now handled in validate_r_code function below - - # Validate R code with interactive approval - is_safe, error = validate_r_code(r_code, context) + # Validate the code and the declared packages together + is_safe, error = validate_r_code(r_code, context, packages=packages) if not is_safe: if error and error.startswith("APPROVAL_NEEDED:"): # Extract package name and request approval @@ -372,9 +346,12 @@ async def execute_r_analysis(context, params) -> dict[str, Any]: full_script, args, include_image=True, + timeout=params.get("timeout_seconds"), ) else: - result = await execute_r_script_async(full_script, args) + result = await execute_r_script_async( + full_script, args, timeout=params.get("timeout_seconds") + ) await context.info("R analysis completed successfully") @@ -523,7 +500,7 @@ async def execute_r_analysis(context, params) -> dict[str, Any]: }, "required": ["success", "operation_type", "action", "message"], }, - description="Approve or deny R operations including file writing (ggsave, write.csv), package installation (install.packages), and system operations. Provides explicit user control over potentially sensitive operations. Session approvals apply only to current analysis session, while permanent approvals persist across sessions. Essential for enabling file saving, package installation, and system interactions while maintaining security through explicit consent.", + description="Approve or deny a pending R operation (file write, package install, or system call). Call this when execute_r_analysis reports approval_required.", ) async def approve_operation(context, params) -> dict[str, Any]: """Universal approval system for R operations.""" @@ -537,9 +514,7 @@ async def approve_operation(context, params) -> dict[str, Any]: f"Processing {action} request for {operation_type}: {specific_operation}" ) - # Initialize approval tracking - if not hasattr(context, "_approved_operations"): - context._approved_operations = {} + approvals = _approved_operations(context) # Get operation category info category_info = OPERATION_CATEGORIES.get(operation_type, {}) @@ -547,23 +522,34 @@ async def approve_operation(context, params) -> dict[str, Any]: if action == "approve": # Store approval - if operation_type not in context._approved_operations: - context._approved_operations[operation_type] = {} + if operation_type not in approvals: + approvals[operation_type] = {} approval_data = { "specific_operation": specific_operation, "scope": scope, - "approved_at": __import__("time").time(), + "approved_at": time.time(), } if directory and operation_type == "file_operations": approval_data["directory"] = directory - # Enable VFS write mode if available - if hasattr(context.lifespan, "vfs") and context.lifespan.vfs: - context.lifespan.vfs.read_only = False - await context.info(f"✅ Enabled file writing to: {directory}") + # Grant write access to this directory only, rather than lifting + # read-only for the whole process. + vfs = getattr(context.lifespan, "vfs", None) + if vfs is not None: + try: + granted = vfs.grant_write(directory) + await context.info(f"✅ Enabled file writing to: {granted}") + except Exception as e: + await context.error( + f"Cannot grant write access to {directory}: {e}" + ) + return { + "success": False, + "error": f"Cannot grant write access to {directory}: {e}", + } - context._approved_operations[operation_type][specific_operation] = approval_data + approvals[operation_type][specific_operation] = approval_data message = f"✅ Approved {operation_type} operation: {specific_operation}" if scope == "session": @@ -594,9 +580,7 @@ async def approve_operation(context, params) -> dict[str, Any]: "action": "approved", "scope": scope, "message": message, - "approved_operations": { - k: list(v.keys()) for k, v in context._approved_operations.items() - }, + "approved_operations": {k: list(v.keys()) for k, v in approvals.items()}, "security_info": security_info, } @@ -611,10 +595,7 @@ async def approve_operation(context, params) -> dict[str, Any]: "action": "denied", "scope": "none", "message": message, - "approved_operations": { - k: list(v.keys()) - for k, v in getattr(context, "_approved_operations", {}).items() - }, + "approved_operations": {k: list(v.keys()) for k, v in approvals.items()}, } @@ -673,7 +654,7 @@ async def approve_operation(context, params) -> dict[str, Any]: }, "required": ["packages", "total_count", "category"], }, - description="Lists comprehensive R packages whitelisted for statistical analysis based on CRAN task views. Covers 700+ packages across machine learning, econometrics, time series, Bayesian methods, survival analysis, spatial data, and more. Use 'summary' to see category breakdown or specific categories to explore available packages. Helps discover capabilities, plan analyses, and verify package availability.", + description="Lists the R packages allowed in execute_r_analysis, by category. Pass 'summary' for the category breakdown.", ) async def list_allowed_r_packages(context, params) -> dict[str, Any]: """List allowed R packages by comprehensive CRAN task view categories.""" @@ -800,7 +781,7 @@ async def list_allowed_r_packages(context, params) -> dict[str, Any]: }, "required": ["success", "package", "action", "message"], }, - description="Approve or deny R packages for use in flexible R code execution. Allows users to grant permission for packages not in the default allowlist. Session-only approval means packages are approved for the current analysis session only. Use this tool when RMCP requests package approval for statistical analysis.", + description="Approve or deny an R package that is not in the default allowlist. Call this when execute_r_analysis reports approval_required.", ) async def approve_r_package(context, params) -> dict[str, Any]: """Handle user approval/denial of R packages.""" @@ -809,11 +790,10 @@ async def approve_r_package(context, params) -> dict[str, Any]: session_only = params.get("session_only", True) # Get or create session package store - if not hasattr(context, "_approved_packages"): - context._approved_packages = set() + approved = _approved_packages(context) if action == "approve": - context._approved_packages.add(package_name) + approved.add(package_name) # Log security approval event log_security_event( @@ -841,7 +821,7 @@ async def approve_r_package(context, params) -> dict[str, Any]: "package": package_name, "action": "approved", "message": message, - "session_packages": list(context._approved_packages), + "session_packages": sorted(approved), } else: # deny @@ -861,5 +841,5 @@ async def approve_r_package(context, params) -> dict[str, Any]: "package": package_name, "action": "denied", "message": f"Package '{package_name}' has been denied. Please modify your analysis to use approved packages.", - "session_packages": list(getattr(context, "_approved_packages", [])), + "session_packages": sorted(approved), } diff --git a/rmcp/tools/formula_builder.py b/rmcp/tools/formula_builder.py index 1494e53..224928c 100644 --- a/rmcp/tools/formula_builder.py +++ b/rmcp/tools/formula_builder.py @@ -111,7 +111,7 @@ ], "additionalProperties": False, }, - description="Converts natural language descriptions into proper R statistical formulas using intelligent pattern matching and variable name recognition. Handles complex relationships including interactions, transformations, and nested terms. Validates variable names against provided datasets and suggests corrections for typos. Use when users describe statistical relationships in plain English rather than formal formula syntax, or to help non-technical users specify models correctly.", + description="Converts a plain-English description of a model into R formula syntax, validating variable names against a dataset.", ) async def build_formula(context, params) -> dict[str, Any]: """Convert natural language to R formula.""" @@ -459,7 +459,7 @@ def _get_formula_examples(analysis_type: str) -> list[dict[str, str]]: "required": ["is_valid", "formula_parsed", "suggestions", "analysis_type"], "additionalProperties": False, }, - description="Validates R formula syntax and checks compatibility with provided datasets including variable existence, data type appropriateness, and formula structure correctness. Identifies missing variables, type mismatches, and syntax errors with specific suggestions for fixes. Essential quality control before running statistical analyzes. Use to catch formula errors early, ensure model specifications are valid, or verify that formulas match intended analytical objectives.", + description="Checks an R formula for syntax errors, and against a dataset for missing or mistyped variables.", ) async def validate_formula(context, params) -> dict[str, Any]: """Validate R formula against data.""" diff --git a/rmcp/tools/helpers.py b/rmcp/tools/helpers.py index 2809181..5d0d8d1 100644 --- a/rmcp/tools/helpers.py +++ b/rmcp/tools/helpers.py @@ -88,7 +88,7 @@ ], "additionalProperties": False, }, - description="Analyzes error messages from statistical operations and provides intelligent, actionable suggestions for fixes including parameter adjustments, data transformations, or alternative approaches. Uses pattern matching and statistical knowledge to diagnose common issues. Use when statistical analyzes fail, to help users understand error causes, debug complex workflows, or learn proper statistical software usage through guided error resolution.", + description="Diagnoses an error message from a failed analysis and suggests concrete fixes.", ) async def suggest_fix(context, params) -> dict[str, Any]: """Analyze error and provide actionable solutions.""" @@ -428,7 +428,7 @@ async def _analyze_data_for_errors(context, data: dict) -> dict[str, Any]: ], "additionalProperties": False, }, - description="Performs comprehensive data quality validation checking for missing values, outliers, data type consistency, range validity, and structural issues that could cause analysis failures. Provides detailed quality reports with severity ratings and remediation suggestions. Use before statistical analyzes to prevent errors, ensure data reliability, meet analysis assumptions, or generate data quality documentation for research compliance.", + description="Checks a dataset for missing values, outliers, type inconsistencies, and range problems before analysis.", ) async def validate_data(context, params) -> dict[str, Any]: """Validate data for analysis and identify potential issues.""" @@ -629,7 +629,7 @@ def _get_analysis_recommendations( ], "additionalProperties": False, }, - description="Loads curated example datasets suitable for demonstrating statistical techniques, testing analysis workflows, or learning RMCP functionality. Includes classic datasets (iris, mtcars, economics) with documentation and suggested analyses. Use for tutorials, testing new analytical approaches, teaching statistical concepts, or exploring RMCP capabilities with known datasets that have well-understood properties and expected results.", + description="Loads a built-in example dataset (iris, mtcars, economics, and others) for testing or demonstration.", ) async def load_example(context, params) -> dict[str, Any]: """Load example datasets for analysis and testing.""" @@ -647,14 +647,7 @@ async def load_example(context, params) -> dict[str, Any]: return result except Exception as e: await context.error("Failed to load example dataset", error=str(e)) - return { - "error": f"Failed to load example dataset: {str(e)}", - "data": {}, - "metadata": { - "name": dataset_name, - "rows": 0, - "columns": 0, - "description": "Failed to load", - }, - "suggested_analyses": [], - } + # Re-raise rather than returning a partial dict: the fallback omitted + # required output properties, so schema validation replaced the real + # error with "'statistics' is a required property". + raise diff --git a/rmcp/tools/introspection.py b/rmcp/tools/introspection.py index 4204f9c..2279906 100644 --- a/rmcp/tools/introspection.py +++ b/rmcp/tools/introspection.py @@ -171,7 +171,7 @@ class = class(obj)[1], try: # Execute with session support if available - result = await context.execute_r_with_session(r_script, args, use_session=True) + result = await context.execute_r(r_script, args) return { "objects": result.get("objects", {}), @@ -363,7 +363,7 @@ class = class(obj), args = {"object_name": object_name} try: - result = await context.execute_r_with_session(r_script, args, use_session=True) + result = await context.execute_r(r_script, args) return {**result, "session_id": session_id, "success": True} @@ -471,7 +471,7 @@ async def list_r_packages(context: Context, params: dict[str, Any]) -> dict[str, args: dict[str, Any] = {} try: - result = await context.execute_r_with_session(r_script, args, use_session=True) + result = await context.execute_r(r_script, args) return {**result, "session_id": session_id, "success": True} @@ -550,20 +550,7 @@ async def get_r_session_info( args: dict[str, Any] = {} try: - result = await context.execute_r_with_session(r_script, args, use_session=True) - - # Add RMCP-specific session info if available - if context.is_r_session_enabled(): - from ..r_session import get_session_manager - - session_manager = get_session_manager() - - rmcp_session_info = await session_manager.get_session_info( - session_id or context.get_r_session_id() or "default" - ) - - if rmcp_session_info: - result["rmcp_session"] = rmcp_session_info + result = await context.execute_r(r_script, args) return {**result, "session_id": session_id, "success": True} diff --git a/rmcp/tools/machine_learning.py b/rmcp/tools/machine_learning.py index 5e84fec..d06faec 100644 --- a/rmcp/tools/machine_learning.py +++ b/rmcp/tools/machine_learning.py @@ -112,7 +112,7 @@ ], "additionalProperties": False, }, - description="Performs K-means clustering to partition data into k clusters based on feature similarity. Uses multiple random starts for optimal clustering and provides cluster assignments, centroids, within-cluster sum of squares, and silhouette analysis for cluster quality assessment. Use for customer segmentation, market research, data exploration, pattern recognition, or reducing data complexity by grouping similar observations.", + description="K-means partitioning into k clusters. Returns assignments, centroids, within-cluster sum of squares, and silhouette scores.", ) async def kmeans_clustering(context, params) -> dict[str, Any]: """Perform K-means clustering.""" @@ -225,7 +225,7 @@ async def kmeans_clustering(context, params) -> dict[str, Any]: ], "additionalProperties": False, }, - description="Builds decision tree models for classification (categorical outcomes) or regression (continuous outcomes) using recursive binary splitting. Provides tree structure, variable importance rankings, prediction rules, and cross-validation accuracy. Trees are interpretable and handle mixed data types naturally. Use for rule-based modeling, feature selection, understanding decision processes, or when interpretability is more important than maximum accuracy.", + description="Single decision tree for classification or regression. Returns tree structure, variable importance, and cross-validated accuracy. Prefer over random_forest when interpretability matters more than accuracy.", ) async def decision_tree(context, params) -> dict[str, Any]: """Build decision tree model.""" @@ -344,7 +344,7 @@ async def decision_tree(context, params) -> dict[str, Any]: ], "additionalProperties": False, }, - description="Constructs Random Forest ensemble models combining multiple decision trees with bootstrap sampling and random feature selection. Provides predictions, variable importance rankings, out-of-bag error estimates, and partial dependence plots. More accurate and robust than single trees while maintaining interpretability through variable importance. Use for high-accuracy prediction, feature selection, handling missing data, or non-linear relationships.", + description="Random forest ensemble for classification or regression. Returns predictions, variable importance, and out-of-bag error.", ) async def random_forest(context, params) -> dict[str, Any]: """Build Random Forest model.""" diff --git a/rmcp/tools/package_security.py b/rmcp/tools/package_security.py deleted file mode 100644 index b7db1c8..0000000 --- a/rmcp/tools/package_security.py +++ /dev/null @@ -1,373 +0,0 @@ -""" -R Package Security and Risk Assessment for RMCP. - -This module provides security assessment and filtering capabilities for R packages, -including risk categorization and filtering based on security criteria. -""" - -from enum import Enum - -from ..logging_config import configure_structured_logging, get_logger - -logger = get_logger(__name__) - - -class SecurityLevel(Enum): - """Security risk levels for R packages.""" - - LOW = "low" - MEDIUM = "medium" - HIGH = "high" - CRITICAL = "critical" - - -class PackageRiskCategory(Enum): - """Categories of package security risks.""" - - SYSTEM_ACCESS = "system_access" - NETWORK_ACCESS = "network_access" - FILE_OPERATIONS = "file_operations" - CODE_EXECUTION = "code_execution" - CRYPTO_SECURITY = "crypto_security" - EXTERNAL_DEPENDENCIES = "external_dependencies" - - -# Packages that typically involve system-level operations -SYSTEM_ACCESS_PACKAGES = { - "R.utils", - "RAppArmor", - "unix", - "processx", - "callr", - "sys", - "RCurl", - "curl", - "httr", - "httr2", - "rvest", - "downloader", - "pingr", - "urltools", - "webutils", - "servr", -} - -# Packages that involve network operations -NETWORK_ACCESS_PACKAGES = { - "RCurl", - "curl", - "httr", - "httr2", - "rvest", - "xml2", - "jsonlite", - "pingr", - "downloader", - "googledrive", - "googlesheets4", - "gh", - "pins", - "plumber", - "httpuv", - "websocket", - "webutils", -} - -# Packages that may involve direct file system operations -FILE_OPERATION_PACKAGES = { - "R.utils", - "tools", - "readr", - "readxl", - "writexl", - "openxlsx", - "haven", - "foreign", - "rio", - "data.table", - "fst", - "feather", - "archive", - "zip", - "tar", - "R.cache", -} - -# Packages involving code execution or compilation -CODE_EXECUTION_PACKAGES = { - "Rcpp", - "RcppArmadillo", - "inline", - "compiler", - "devtools", - "remotes", - "pak", - "rstan", - "rstanarm", - "tensorflow", - "torch", - "reticulate", - "JuliaCall", - "RJulia", - "V8", - "rJava", -} - -# Packages with external system dependencies -EXTERNAL_DEPENDENCY_PACKAGES = { - "rJava", - "RMySQL", - "RPostgreSQL", - "ROracle", - "RODBC", - "odbc", - "Cairo", - "tkrplot", - "tcltk", - "Rgtk2", - "cairoDevice", - "rgl", - "rgdal", - "rgeos", - "sf", - "terra", - "gdalUtils", -} - - -def assess_package_security_risk( - package_name: str, -) -> tuple[SecurityLevel, list[PackageRiskCategory]]: - """ - Assess the security risk level and categories for an R package. - - Args: - package_name: Name of the R package - - Returns: - Tuple of (SecurityLevel, List of risk categories) - """ - risks = [] - - if package_name in SYSTEM_ACCESS_PACKAGES: - risks.append(PackageRiskCategory.SYSTEM_ACCESS) - - if package_name in NETWORK_ACCESS_PACKAGES: - risks.append(PackageRiskCategory.NETWORK_ACCESS) - - if package_name in FILE_OPERATION_PACKAGES: - risks.append(PackageRiskCategory.FILE_OPERATIONS) - - if package_name in CODE_EXECUTION_PACKAGES: - risks.append(PackageRiskCategory.CODE_EXECUTION) - - if package_name in EXTERNAL_DEPENDENCY_PACKAGES: - risks.append(PackageRiskCategory.EXTERNAL_DEPENDENCIES) - - # Determine overall security level - if ( - PackageRiskCategory.SYSTEM_ACCESS in risks - or PackageRiskCategory.CODE_EXECUTION in risks - ): - level = SecurityLevel.HIGH - elif PackageRiskCategory.NETWORK_ACCESS in risks or len(risks) >= 2: - level = SecurityLevel.MEDIUM - elif risks: - level = SecurityLevel.LOW - else: - level = SecurityLevel.LOW # Default for statistical packages - - return level, risks - - -def filter_packages_by_security( - packages: set[str], max_security_level: SecurityLevel -) -> set[str]: - """ - Filter packages based on maximum allowed security level. - - Args: - packages: Set of package names to filter - max_security_level: Maximum allowed security level - - Returns: - Filtered set of packages - """ - security_order = [ - SecurityLevel.LOW, - SecurityLevel.MEDIUM, - SecurityLevel.HIGH, - SecurityLevel.CRITICAL, - ] - max_index = security_order.index(max_security_level) - - filtered = set() - for package in packages: - level, _ = assess_package_security_risk(package) - if security_order.index(level) <= max_index: - filtered.add(package) - - return filtered - - -def get_security_report(packages: set[str]) -> dict: - """ - Generate a security assessment report for a set of packages. - - Args: - packages: Set of package names to assess - - Returns: - Dictionary with security statistics and categorization - """ - security_stats = dict.fromkeys(SecurityLevel, 0) - risk_stats = dict.fromkeys(PackageRiskCategory, 0) - flagged_packages = {level: [] for level in SecurityLevel} - - for package in packages: - level, risks = assess_package_security_risk(package) - security_stats[level] += 1 - flagged_packages[level].append(package) - - for risk in risks: - risk_stats[risk] += 1 - - return { - "total_packages": len(packages), - "security_levels": { - level.value: count for level, count in security_stats.items() - }, - "risk_categories": {risk.value: count for risk, count in risk_stats.items()}, - "flagged_packages": { - level.value: sorted(pkgs) for level, pkgs in flagged_packages.items() - }, - "recommendations": _get_security_recommendations(security_stats), - } - - -def _get_security_recommendations( - security_stats: dict[SecurityLevel, int], -) -> list[str]: - """Generate security recommendations based on package statistics.""" - recommendations = [] - - if security_stats[SecurityLevel.HIGH] > 0: - recommendations.append( - f"HIGH RISK: {security_stats[SecurityLevel.HIGH]} packages with elevated privileges. " - "Consider approval workflow for system access packages." - ) - - if security_stats[SecurityLevel.MEDIUM] > 10: - recommendations.append( - f"MEDIUM RISK: {security_stats[SecurityLevel.MEDIUM]} packages with network/file access. " - "Monitor for potential data exfiltration." - ) - - if security_stats[SecurityLevel.LOW] > 0: - recommendations.append( - f"LOW RISK: {security_stats[SecurityLevel.LOW]} standard statistical packages. " - "Generally safe for analytical work." - ) - - return recommendations - - -# Popular packages based on CRAN download statistics (for prioritization) -TOP_DOWNLOADED_PACKAGES = { - # Top 50 most downloaded packages (weekly stats) - "rlang", - "cli", - "ggplot2", - "vctrs", - "lifecycle", - "dplyr", - "ragg", - "textshaping", - "tidyselect", - "devtools", - "glue", - "tibble", - "fansi", - "utf8", - "pillar", - "digest", - "crayon", - "withr", - "R6", - "magrittr", - "ellipsis", - "pkgconfig", - "stringi", - "tidyr", - "readr", - "purrr", - "forcats", - "lubridate", - "stringr", - "broom", - "scales", - "colorspace", - "munsell", - "RColorBrewer", - "viridisLite", - "ggrepel", - "corrplot", - "cowplot", - "gridExtra", - "lattice", - "MASS", - "survival", - "Matrix", - "nlme", - "mgcv", - "boot", - "cluster", - "foreign", - "nnet", - "rpart", -} - - -def prioritize_packages_by_popularity(packages: set[str]) -> list[str]: - """ - Sort packages by popularity (download statistics). - - Args: - packages: Set of package names - - Returns: - List of packages sorted by popularity (most popular first) - """ - popular = [] - standard = [] - - for package in packages: - if package in TOP_DOWNLOADED_PACKAGES: - popular.append(package) - else: - standard.append(package) - - return sorted(popular) + sorted(standard) - - -if __name__ == "__main__": - # Example usage - from package_whitelist_comprehensive import get_comprehensive_package_whitelist - - packages = get_comprehensive_package_whitelist() - report = get_security_report(packages) - - # Configure structured logging for utility - configure_structured_logging(level="INFO", development_mode=True) - - logger.info( - "Security Assessment Report", - report_type="security_assessment", - total_packages=report["total_packages"], - security_levels=report["security_levels"], - risk_categories=report["risk_categories"], - ) - - logger.info( - "Security assessment completed", recommendations=report["recommendations"] - ) diff --git a/rmcp/tools/package_tiers.py b/rmcp/tools/package_tiers.py deleted file mode 100644 index 091c177..0000000 --- a/rmcp/tools/package_tiers.py +++ /dev/null @@ -1,436 +0,0 @@ -""" -Tiered R Package Permission System for RMCP. - -This module implements a tiered approach to R package permissions, balancing -security with usability by categorizing packages into permission tiers. -""" - -from enum import Enum - -from ..logging_config import configure_structured_logging, get_logger - -logger = get_logger(__name__) - -from .package_security import ( - TOP_DOWNLOADED_PACKAGES, - SecurityLevel, - assess_package_security_risk, -) -from .package_whitelist_comprehensive import ( - BASE_R_PACKAGES, - BAYESIAN_INFERENCE, - CORE_INFRASTRUCTURE, - ECONOMETRICS, - MACHINE_LEARNING, - OPTIMIZATION, - SPATIAL_ANALYSIS, - SURVIVAL_ANALYSIS, - TIDYVERSE_ECOSYSTEM, - TIME_SERIES, -) - - -class PackageTier(Enum): - """Package permission tiers for different security and approval levels.""" - - AUTO_APPROVED = "auto_approved" # Tier 1: Automatically approved, core packages - USER_APPROVAL = "user_approval" # Tier 2: Requires user approval, extended packages - ADMIN_APPROVAL = ( - "admin_approval" # Tier 3: Requires admin approval, specialized packages - ) - BLOCKED = "blocked" # Tier 4: Blocked packages, security risks - - -def get_package_tier(package_name: str) -> PackageTier: - """ - Determine the permission tier for a given R package. - - Args: - package_name: Name of the R package - - Returns: - PackageTier enum value - """ - # Assess security risk - security_level, risk_categories = assess_package_security_risk(package_name) - - # Tier 1: Auto-approved (core statistical packages) - if package_name in get_tier1_packages(): - return PackageTier.AUTO_APPROVED - - # Tier 4: Blocked (high security risk packages not in whitelist) - if ( - security_level == SecurityLevel.HIGH - and package_name in get_high_risk_packages() - ): - return PackageTier.BLOCKED - - # Tier 3: Admin approval (specialized or risky packages) - if security_level == SecurityLevel.HIGH or package_name in get_tier3_packages(): - return PackageTier.ADMIN_APPROVAL - - # Tier 2: User approval (extended packages) - return PackageTier.USER_APPROVAL - - -def get_tier1_packages() -> set[str]: - """ - Get Tier 1 packages: Auto-approved core packages. - - Returns: - Set of package names that are automatically approved - """ - # Core packages that are essential and low-risk - tier1 = set() - - # Base R packages (always safe) - tier1.update(BASE_R_PACKAGES) - - # Core infrastructure (essential for R ecosystem) - core_safe = { - "rlang", - "cli", - "glue", - "magrittr", - "crayon", - "pillar", - "fansi", - "utf8", - "lifecycle", - "vctrs", - "ellipsis", - "digest", - "tibble", - } - tier1.update(core_safe & CORE_INFRASTRUCTURE) - - # Essential tidyverse (data manipulation core) - tidyverse_core = { - "dplyr", - "tidyr", - "ggplot2", - "readr", - "tibble", - "stringr", - "forcats", - "lubridate", - "purrr", - "broom", - } - tier1.update(tidyverse_core & TIDYVERSE_ECOSYSTEM) - - # Essential statistical packages - stats_core = { - "MASS", - "survival", - "boot", - "cluster", - "lattice", - "Matrix", - "nlme", - "mgcv", - "foreign", - "nnet", - "rpart", - } - tier1.update(stats_core) - - # Popular visualization - viz_safe = {"scales", "colorspace", "RColorBrewer", "viridisLite", "corrplot"} - tier1.update(viz_safe) - - return tier1 - - -def get_tier2_packages() -> set[str]: - """ - Get Tier 2 packages: User approval required. - - Returns: - Set of package names requiring user approval - """ - # Most comprehensive packages that are medium risk - tier2 = set() - - # Extended tidyverse - tier2.update(TIDYVERSE_ECOSYSTEM - get_tier1_packages()) - - # Core machine learning (popular, well-established) - ml_popular = { - "caret", - "randomForest", - "rpart", - "tree", - "e1071", - "cluster", - "glmnet", - "gbm", - "BART", - "ranger", - "mlr3", - "tidymodels", - } - tier2.update(ml_popular & MACHINE_LEARNING) - - # Core econometrics - econ_popular = { - "AER", - "plm", - "lmtest", - "sandwich", - "car", - "systemfit", - "vars", - "forecast", - "urca", - "quantreg", - } - tier2.update(econ_popular & ECONOMETRICS) - - # Core time series - ts_popular = { - "zoo", - "xts", - "forecast", - "tseries", - "urca", - "vars", - "fable", - "feasts", - "tsibble", - } - tier2.update(ts_popular & TIME_SERIES) - - # Core Bayesian (established packages) - bayes_popular = {"coda", "MCMCpack", "BayesFactor", "arm", "boa", "rstan", "brms"} - tier2.update(bayes_popular & BAYESIAN_INFERENCE) - - # Popular packages (high download count = community validated) - popular_safe = TOP_DOWNLOADED_PACKAGES & ( - MACHINE_LEARNING | ECONOMETRICS | TIME_SERIES | SURVIVAL_ANALYSIS - ) - tier2.update(popular_safe) - - return tier2 - - -def get_tier3_packages() -> set[str]: - """ - Get Tier 3 packages: Admin approval required. - - Returns: - Set of package names requiring admin approval - """ - # Specialized or higher-risk packages - tier3 = set() - - # Advanced/specialized packages - tier3.update(SPATIAL_ANALYSIS) # GIS packages often have external dependencies - tier3.update(OPTIMIZATION) # Mathematical optimization - - # Advanced ML packages (less common, more specialized) - ml_advanced = { - "xgboost", - "lightgbm", - "h2o", - "tensorflow", - "torch", - "keras", - "deepnet", - "RSNNS", - "kernlab", - } - tier3.update(ml_advanced & MACHINE_LEARNING) - - # Advanced Bayesian (MCMC samplers, specialized) - bayes_advanced = { - "rstan", - "rstanarm", - "brms", - "nimble", - "rjags", - "R2WinBUGS", - "LaplacesDemon", - "BayesianTools", - } - tier3.update(bayes_advanced & BAYESIAN_INFERENCE) - - # Development and system packages - dev_packages = { - "devtools", - "remotes", - "pak", - "usethis", - "pkgbuild", - "pkgload", - "rcmdcheck", - "roxygen2", - } - tier3.update(dev_packages) - - # Network and web packages - web_packages = { - "httr", - "httr2", - "curl", - "RCurl", - "rvest", - "xml2", - "jsonlite", - "plumber", - "shiny", - "httpuv", - } - tier3.update(web_packages) - - return tier3 - - -def get_high_risk_packages() -> set[str]: - """ - Get packages considered high-risk for security. - - Returns: - Set of package names with elevated security risks - """ - return { - # System access - "RAppArmor", - "unix", - "processx", - "callr", - "sys", - # Code execution and compilation - "Rcpp", - "RcppArmadillo", - "inline", - "compiler", - "reticulate", - "JuliaCall", - "RJulia", - "V8", - "rJava", - # External system dependencies - "RMySQL", - "RPostgreSQL", - "ROracle", - "RODBC", - "Cairo", - "tkrplot", - "tcltk", - "Rgtk2", - "cairoDevice", - "rgl", - "rgdal", - "rgeos", - } - - -def get_packages_by_tier() -> dict[PackageTier, set[str]]: - """ - Get packages organized by permission tier. - - Returns: - Dictionary mapping tiers to package sets - """ - return { - PackageTier.AUTO_APPROVED: get_tier1_packages(), - PackageTier.USER_APPROVAL: get_tier2_packages(), - PackageTier.ADMIN_APPROVAL: get_tier3_packages(), - PackageTier.BLOCKED: get_high_risk_packages(), - } - - -def get_tier_statistics() -> dict: - """ - Get statistics about package distribution across tiers. - - Returns: - Dictionary with tier statistics - """ - tiers = get_packages_by_tier() - - stats = {} - for tier, packages in tiers.items(): - stats[tier.value] = { - "count": len(packages), - "examples": sorted(packages)[:5], - } - - total_packages = sum(len(packages) for packages in tiers.values()) - stats["total"] = total_packages - - return stats - - -def check_package_permission(package_name: str) -> dict: - """ - Check the permission requirements for a specific package. - - Args: - package_name: Name of the R package - - Returns: - Dictionary with permission information - """ - tier = get_package_tier(package_name) - security_level, risk_categories = assess_package_security_risk(package_name) - - return { - "package": package_name, - "tier": tier.value, - "security_level": security_level.value, - "risk_categories": [risk.value for risk in risk_categories], - "auto_approved": tier == PackageTier.AUTO_APPROVED, - "requires_approval": tier - in [PackageTier.USER_APPROVAL, PackageTier.ADMIN_APPROVAL], - "blocked": tier == PackageTier.BLOCKED, - "description": _get_tier_description(tier), - } - - -def _get_tier_description(tier: PackageTier) -> str: - """Get human-readable description of a permission tier.""" - descriptions = { - PackageTier.AUTO_APPROVED: "Core package, automatically approved for use", - PackageTier.USER_APPROVAL: "Extended package, requires user approval", - PackageTier.ADMIN_APPROVAL: "Specialized package, requires admin approval", - PackageTier.BLOCKED: "High-risk package, blocked for security", - } - return descriptions[tier] - - -if __name__ == "__main__": - # Configure structured logging for utility - configure_structured_logging(level="INFO", development_mode=True) - - # Print tier statistics using structured logging - stats = get_tier_statistics() - - logger.info( - "Package Tier Statistics", - report_type="tier_statistics", - total_packages=stats["total"], - ) - - for tier_name, tier_stats in stats.items(): - if tier_name != "total": - logger.info( - "Tier details", - tier=tier_name.upper(), - count=tier_stats["count"], - examples=tier_stats["examples"], - ) - - # Test specific packages - test_packages = ["ggplot2", "xgboost", "devtools", "rJava"] - logger.info("Package Permission Examples") - - for pkg in test_packages: - info = check_package_permission(pkg) - logger.info( - "Package permission check", - package=pkg, - tier=info["tier"], - description=info["description"], - ) diff --git a/rmcp/tools/regression.py b/rmcp/tools/regression.py index 44d04ce..f61cd86 100644 --- a/rmcp/tools/regression.py +++ b/rmcp/tools/regression.py @@ -117,7 +117,7 @@ "required": ["coefficients", "r_squared", "n_obs", "method"], "additionalProperties": False, }, - description="Performs ordinary least squares (OLS) linear regression to model relationships between a dependent variable and one or more predictors. Returns coefficients with standard errors, confidence intervals, R-squared, F-statistic, and comprehensive diagnostic statistics including residuals and fitted values. Use for prediction, inference, understanding linear relationships, or testing hypotheses about continuous outcomes. Handles missing values and supports weighted observations.", + description="Ordinary least squares regression. Returns coefficients with standard errors and confidence intervals, R-squared, F-statistic, residuals, and fitted values.", ) async def linear_model(context, params) -> dict[str, Any]: """ @@ -276,7 +276,7 @@ async def linear_model(context, params) -> dict[str, Any]: "required": ["correlation_matrix", "method", "variables"], "additionalProperties": False, }, - description="Computes pairwise correlation matrix between numeric variables using Pearson, Spearman, or Kendall methods. Provides correlation coefficients, significance tests (p-values), and handles missing data appropriately. Use to explore relationships between variables, identify multicollinearity, or understand data structure before modeling. Pearson for linear relationships, Spearman for monotonic non-linear relationships, Kendall for small samples or ties.", + description="Pairwise correlation matrix with significance tests. Pearson for linear, Spearman for monotonic, Kendall for small samples or many ties.", ) async def correlation_analysis(context, params) -> dict[str, Any]: """ @@ -455,7 +455,7 @@ async def correlation_analysis(context, params) -> dict[str, Any]: ], "additionalProperties": False, }, - description="Fits generalized linear models (GLM) including logistic regression for binary outcomes, Poisson regression for count data, and other exponential family distributions. Returns coefficients, odds ratios (for logistic), AIC/BIC for model comparison, and diagnostic statistics. Use for binary classification, count modeling, or when dependent variable doesn't follow normal distribution. Includes prediction probabilities and confusion matrix for classification tasks.", + description="Generalized linear models: logistic for binary outcomes, Poisson for counts, and other exponential-family distributions. Returns coefficients, odds ratios, and AIC/BIC.", ) async def logistic_regression(context, params) -> dict[str, Any]: """Fit logistic regression model.""" diff --git a/rmcp/tools/statistical_tests.py b/rmcp/tools/statistical_tests.py index 2ea0f86..adf101f 100644 --- a/rmcp/tools/statistical_tests.py +++ b/rmcp/tools/statistical_tests.py @@ -127,7 +127,7 @@ ], "additionalProperties": False, }, - description="Performs Student's t-tests to compare means: one-sample (test if mean equals hypothesized value), two-sample (compare means between groups), or paired (compare before/after measurements). Returns t-statistic, degrees of freedom, p-value, confidence intervals, and effect size. Use for hypothesis testing about population means, comparing group differences, or analyzing experimental results. Handles equal/unequal variances and provides Cohen's d effect size.", + description="One-sample, two-sample, or paired t-test. Returns the t-statistic, df, p-value, confidence interval, and Cohen's d.", ) async def t_test(context, params) -> dict[str, Any]: """Perform t-test analysis.""" @@ -236,7 +236,7 @@ async def t_test(context, params) -> dict[str, Any]: "required": ["anova_table", "model_summary", "formula", "anova_type"], "additionalProperties": False, }, - description="Performs Analysis of Variance (ANOVA) to test for significant differences between group means. Supports one-way ANOVA (single factor), two-way ANOVA (two factors with interaction), and repeated measures designs. Returns F-statistics, p-values, effect sizes (eta-squared), and post-hoc comparisons when significant. Use when comparing means across 3+ groups, testing factorial designs, or analyzing experimental data with multiple conditions.", + description="One-way, two-way, or repeated-measures ANOVA. Returns F-statistics, p-values, eta-squared, and post-hoc comparisons.", ) async def anova(context, params) -> dict[str, Any]: """Perform ANOVA analysis.""" @@ -355,7 +355,7 @@ async def anova(context, params) -> dict[str, Any]: ], "additionalProperties": False, }, - description="Performs chi-square tests for categorical data analysis: test of independence (relationship between two categorical variables) and goodness-of-fit (whether data follows expected distribution). Returns chi-square statistic, degrees of freedom, p-value, expected frequencies, and standardized residuals. Use for analyzing contingency tables, testing associations between categorical variables, or validating theoretical distributions against observed data.", + description="Chi-square test of independence or goodness-of-fit. Returns the statistic, df, p-value, expected frequencies, and standardized residuals.", ) async def chi_square_test(context, params) -> dict[str, Any]: """Perform chi-square tests.""" @@ -450,7 +450,7 @@ async def chi_square_test(context, params) -> dict[str, Any]: ], "additionalProperties": False, }, - description="Tests variables for normal distribution using multiple methods: Shapiro-Wilk (most powerful for small samples), Kolmogorov-Smirnov, Anderson-Darling, or Jarque-Bera tests. Returns test statistics, p-values, and clear interpretation for each test. Use before parametric statistical analyzes, to validate model assumptions, or to choose appropriate statistical methods. Critical for regression diagnostics and assumption checking.", + description="Tests normality via Shapiro-Wilk, Kolmogorov-Smirnov, Anderson-Darling, or Jarque-Bera. Shapiro-Wilk is most powerful for small samples.", ) async def normality_test(context, params) -> dict[str, Any]: """Test for normality.""" diff --git a/rmcp/tools/timeseries.py b/rmcp/tools/timeseries.py index ce6452d..5cc1781 100644 --- a/rmcp/tools/timeseries.py +++ b/rmcp/tools/timeseries.py @@ -125,7 +125,7 @@ ], "additionalProperties": False, }, - description="Fits ARIMA (AutoRegressive Integrated Moving Average) models for time series forecasting and analysis. Automatically selects optimal model parameters or uses specified orders, generates point forecasts with confidence intervals, and provides model diagnostics including AIC/BIC. Use for forecasting future values, understanding temporal patterns, or modeling autocorrelated data. Handles both non-seasonal and seasonal ARIMA models with integrated differencing for non-stationary series.", + description="Fits ARIMA models for forecasting, with automatic or specified orders. Returns forecasts with intervals plus AIC/BIC. Handles seasonal series.", ) async def arima_model(context, params) -> dict[str, Any]: """Fit ARIMA model and generate forecasts.""" @@ -217,7 +217,7 @@ async def arima_model(context, params) -> dict[str, Any]: ], "additionalProperties": False, }, - description="Decomposes time series into constituent components: trend (long-term movement), seasonal (repeating patterns), and remainder (irregular fluctuations). Supports both additive and multiplicative decomposition methods. Use to understand underlying patterns, identify seasonal effects, detect structural changes, or prepare data for forecasting. Essential for exploratory time series analysis and identifying appropriate modeling approaches.", + description="Splits a time series into trend, seasonal, and remainder components (additive or multiplicative).", ) async def decompose_timeseries(context, params) -> dict[str, Any]: """Decompose time series into components.""" @@ -297,7 +297,7 @@ async def decompose_timeseries(context, params) -> dict[str, Any]: ], "additionalProperties": False, }, - description="Tests time series for stationarity using Augmented Dickey-Fuller (ADF), Kwiatkowski-Phillips-Schmidt-Shin (KPSS), or Phillips-Perron tests. Stationarity is required for many time series models like ARIMA. Returns test statistics, p-values, critical values, and clear interpretation. Use before time series modeling to determine if differencing is needed, or to verify model assumptions. ADF tests for unit roots, KPSS tests trend stationarity.", + description="Tests stationarity via ADF, KPSS, or Phillips-Perron. ADF tests for a unit root, KPSS for trend stationarity. Run before ARIMA to decide whether to difference.", ) async def stationarity_test(context, params) -> dict[str, Any]: """Test time series stationarity.""" diff --git a/rmcp/tools/transforms.py b/rmcp/tools/transforms.py index 47fd39d..7495976 100644 --- a/rmcp/tools/transforms.py +++ b/rmcp/tools/transforms.py @@ -53,7 +53,7 @@ "required": ["data", "variables_created", "n_obs", "operation"], "additionalProperties": False, }, - description="Creates lagged (past values) and lead (future values) variables for time series analysis and panel data. Supports multiple lags/leads simultaneously and handles missing values appropriately. Essential for autoregressive models, studying temporal dependencies, creating predictor variables from time series, or analyzing causality relationships. Use for ARIMA preprocessing, econometric modeling, or feature engineering in time-dependent data.", + description="Creates lagged (past) and lead (future) variables for time series or panel data.", ) async def lag_lead(context, params) -> dict[str, Any]: """Create lagged and lead variables.""" @@ -137,7 +137,7 @@ async def lag_lead(context, params) -> dict[str, Any]: ], "additionalProperties": False, }, - description="Winsorizes variables by replacing extreme values with specified percentiles to reduce outlier impact while preserving data structure. Sets values below lower percentile to that percentile value and values above upper percentile to that percentile value. More robust than trimming since it retains all observations. Use for outlier treatment in regression analysis, robust statistical modeling, or preparing data for parametric analyzes sensitive to extreme values.", + description="Caps extreme values at given percentiles instead of dropping them. Retains all observations, unlike trimming.", ) async def winsorize(context, params) -> dict[str, Any]: """Winsorize variables to handle outliers.""" @@ -206,7 +206,7 @@ async def winsorize(context, params) -> dict[str, Any]: ], "additionalProperties": False, }, - description="Computes first differences, seasonal differences, or higher-order differences to transform non-stationary time series into stationary ones. Essential preprocessing step for ARIMA modeling and many econometric analyzes. Returns differenced series and handles missing values created by differencing. Use to remove trends, achieve stationarity for time series models, or analyze period-to-period changes in economic and financial data.", + description="First, seasonal, or higher-order differencing to make a series stationary. Standard preprocessing before ARIMA.", ) async def difference(context, params) -> dict[str, Any]: """Compute differences of variables.""" @@ -281,7 +281,7 @@ async def difference(context, params) -> dict[str, Any]: ], "additionalProperties": False, }, - description="Standardizes variables using multiple scaling methods: z-score normalization (mean=0, sd=1), min-max scaling (range 0-1), or robust scaling (median=0, MAD=1). Essential for machine learning algorithms, principal component analysis, or when combining variables with different units. Handles missing values and provides scaling parameters for inverse transformation. Use before clustering, neural networks, or any analysis requiring comparable variable scales.", + description="Rescales variables by z-score, min-max, or robust (median/MAD) scaling. Returns the scaling parameters needed to invert the transform.", ) async def standardize(context, params) -> dict[str, Any]: """Standardize variables.""" diff --git a/rmcp/tools/visualization.py b/rmcp/tools/visualization.py index dc9b7d4..bba16d1 100644 --- a/rmcp/tools/visualization.py +++ b/rmcp/tools/visualization.py @@ -89,11 +89,14 @@ }, "required": ["plot_type", "variables"], }, - description="Creates scatter plots to visualize relationships between two continuous variables with optional grouping by categorical variables. Supports trend lines, confidence bands, correlation annotations, and custom styling. Returns base64-encoded images for inline display. Use for exploring correlations, identifying patterns, detecting outliers, comparing groups, or presenting bivariate relationships in reports and presentations.", + description="Scatter plot of two continuous variables, with optional grouping, trend line, and confidence band. Returns a base64 PNG.", ) async def scatter_plot(context, params) -> dict[str, Any]: """Create scatter plot.""" await context.info("Creating scatter plot") + # file_path is optional here; confine it only when a plot is being saved. + if params.get("file_path"): + context.require_write_path(params["file_path"]) r_script = get_r_script("visualization", "scatter_plot") try: # Use the new image-enabled function @@ -194,11 +197,14 @@ async def scatter_plot(context, params) -> dict[str, Any]: }, "required": ["plot_type", "variable", "bins", "statistics", "n_obs"], }, - description="Creates histograms to visualize distributions of continuous variables with optional density overlays, grouping, and statistical annotations. Supports customizable bins, multiple groups with transparency, and normal distribution overlay. Use for understanding data distributions, checking normality assumptions, comparing group distributions, identifying skewness or multimodality, or initial data exploration.", + description="Histogram of a continuous variable, with optional density or normal overlay and grouping. Returns a base64 PNG.", ) async def histogram(context, params) -> dict[str, Any]: """Create histogram.""" await context.info("Creating histogram") + # file_path is optional here; confine it only when a plot is being saved. + if params.get("file_path"): + context.require_write_path(params["file_path"]) r_script = get_r_script("visualization", "histogram") try: # Use the new image-enabled function @@ -293,11 +299,14 @@ async def histogram(context, params) -> dict[str, Any]: }, "required": ["plot_type", "variable", "summary_statistics"], }, - description="Creates box plots (box-and-whisker plots) to display distribution summaries showing median, quartiles, and outliers with optional grouping by categorical variables. Includes notches for median confidence intervals and customizable outlier detection. Use for comparing distributions between groups, identifying outliers, understanding data spread, or presenting distribution summaries in a compact visual format.", + description="Box-and-whisker plot of a distribution, optionally grouped by a categorical variable. Returns a base64 PNG.", ) async def boxplot(context, params) -> dict[str, Any]: """Create box plot.""" await context.info("Creating box plot") + # file_path is optional here; confine it only when a plot is being saved. + if params.get("file_path"): + context.require_write_path(params["file_path"]) r_script = get_r_script("visualization", "boxplot") try: # Use the new image-enabled function @@ -442,11 +451,14 @@ async def boxplot(context, params) -> dict[str, Any]: }, "required": ["plot_type", "statistics", "has_dates", "show_trend"], }, - description="Creates time series plots to visualize temporal patterns in data with optional trend lines, seasonal decomposition overlays, and forecasting extensions. Supports multiple series, custom date formatting, and trend analysis. Use for identifying temporal patterns, detecting seasonality, visualizing forecasts, monitoring trends over time, or presenting time-dependent data in business and research contexts.", + description="Line plot of one or more series over time, with optional trend line. Returns a base64 PNG.", ) async def time_series_plot(context, params) -> dict[str, Any]: """Create time series plot.""" await context.info("Creating time series plot") + # file_path is optional here; confine it only when a plot is being saved. + if params.get("file_path"): + context.require_write_path(params["file_path"]) r_script = get_r_script("visualization", "time_series_plot") try: # Use the new image-enabled function @@ -548,11 +560,14 @@ async def time_series_plot(context, params) -> dict[str, Any]: "n_variables", ], }, - description="Creates correlation heatmap matrices to visualize pairwise correlations between multiple variables using color-coded cells. Supports hierarchical clustering of variables, customizable color schemes, correlation coefficient annotations, and significance indicators. Use for exploring multicollinearity, understanding variable relationships, feature selection, or presenting correlation structure in multivariate data analysis.", + description="Color-coded correlation matrix, optionally hierarchically clustered. Returns a base64 PNG.", ) async def correlation_heatmap(context, params) -> dict[str, Any]: """Create correlation heatmap.""" await context.info("Creating correlation heatmap") + # file_path is optional here; confine it only when a plot is being saved. + if params.get("file_path"): + context.require_write_path(params["file_path"]) r_script = get_r_script("visualization", "correlation_heatmap") try: # Use the new image-enabled function @@ -680,11 +695,14 @@ async def correlation_heatmap(context, params) -> dict[str, Any]: "n_obs", ], }, - description="Creates comprehensive regression diagnostic plots including residuals vs fitted values, Q-Q plots for normality, scale-location plots for homoscedasticity, and Cook's distance for influential observations. Essential for validating regression assumptions and identifying model problems. Use after fitting regression models to check assumptions, identify outliers, detect heteroscedasticity, or validate model appropriateness for the data.", + description="Four-panel regression diagnostics: residuals vs fitted, Q-Q, scale-location, and Cook's distance. Returns a base64 PNG.", ) async def regression_plot(context, params) -> dict[str, Any]: """Create regression diagnostic plots.""" await context.info("Creating regression plots") + # file_path is optional here; confine it only when a plot is being saved. + if params.get("file_path"): + context.require_write_path(params["file_path"]) r_script = get_r_script("visualization", "regression_plot") try: # Use the new image-enabled function diff --git a/rmcp/transport/sdk.py b/rmcp/transport/sdk.py index b174af3..2541205 100644 --- a/rmcp/transport/sdk.py +++ b/rmcp/transport/sdk.py @@ -10,6 +10,7 @@ import contextlib import logging +import re from collections.abc import AsyncIterator from pathlib import Path from typing import TYPE_CHECKING, Any @@ -32,6 +33,37 @@ logger = logging.getLogger(__name__) +#: Request headers used by MCP Streamable HTTP clients. +_CORS_ALLOW_HEADERS = [ + "Accept", + "Authorization", + "Content-Type", + "Last-Event-ID", + "MCP-Protocol-Version", + "Mcp-Session-Id", +] + + +def _split_cors_origins(origins: list[str]) -> tuple[list[str], str | None]: + """Split configured origins into exact values and a regex for wildcards. + + Starlette's ``CORSMiddleware`` compares ``allow_origins`` by equality, so + patterns like ``http://localhost:*`` never match unless they are compiled + into ``allow_origin_regex``. ``*`` expands to ``[^/]*`` so a wildcard port + cannot swallow a path or a different host. + """ + exact: list[str] = [] + patterns: list[str] = [] + for origin in origins: + if origin == "*" or "*" not in origin: + exact.append(origin) + else: + patterns.append( + "".join(r"[^/]*" if ch == "*" else re.escape(ch) for ch in origin) + ) + return exact, "|".join(patterns) if patterns else None + + async def run_stdio(rmcp_server: MCPServer) -> None: """Run the server over stdio using the official SDK transport.""" adapter = build_sdk_server(rmcp_server) @@ -120,7 +152,8 @@ def create_streamable_http_app( Args: rmcp_server: Configured RMCP server with registered tools. api_keys: Bearer tokens accepted on /mcp. Empty/None disables auth. - cors_origins: Allowed CORS origins (defaults to ``["*"]``). + cors_origins: Allowed CORS origins, ``*`` wildcards permitted + (defaults to ``["*"]``). json_response: Return plain JSON instead of SSE streams. stateless: Run sessions statelessly (fresh transport per request). manage_server_lifecycle: Run rmcp startup/shutdown with the app lifespan. @@ -158,12 +191,14 @@ async def lifespan(app: Starlette) -> AsyncIterator[None]: if manage_server_lifecycle: await rmcp_server.shutdown() + allow_origins, allow_origin_regex = _split_cors_origins(cors_origins or ["*"]) middleware = [ Middleware( CORSMiddleware, - allow_origins=cors_origins or ["*"], + allow_origins=allow_origins, + allow_origin_regex=allow_origin_regex, allow_methods=["GET", "POST", "DELETE", "OPTIONS"], - allow_headers=["*"], + allow_headers=_CORS_ALLOW_HEADERS, expose_headers=["Mcp-Session-Id"], ), Middleware(BearerAuthMiddleware, api_keys=api_keys or set()), diff --git a/tests/integration/tools/test_r_concurrency.py b/tests/integration/tools/test_r_concurrency.py new file mode 100644 index 0000000..e8ed69e --- /dev/null +++ b/tests/integration/tools/test_r_concurrency.py @@ -0,0 +1,83 @@ +#!/usr/bin/env python3 +"""Concurrency limiting for R subprocesses. + +Regression cover for a module-level ``asyncio.Semaphore`` that bound itself to +whichever event loop first made a caller wait, then raised "bound to a different +event loop" in every loop afterwards. +""" + +import asyncio +from shutil import which + +import pytest + +pytestmark = pytest.mark.skipif( + which("R") is None, reason="R binary is required for concurrency tests" +) + +from rmcp.config import get_config +from rmcp.r_integration import execute_r_script_async, get_r_semaphore + +SCRIPT = "result <- list(value = 1)" + + +def _oversubscribed() -> int: + """More concurrent calls than permits, so some must queue and wait.""" + return get_config().r.max_concurrent + 2 + + +def test_semaphore_survives_a_second_event_loop(): + """Queueing in one loop must not poison the next one. + + Waiting is what binds the semaphore, so each round has to oversubscribe. + """ + + async def round_trip(): + results = await asyncio.gather( + *[execute_r_script_async(SCRIPT, {}) for _ in range(_oversubscribed())], + return_exceptions=True, + ) + return [r for r in results if isinstance(r, BaseException)] + + first = asyncio.run(round_trip()) + assert not first, f"errors in first loop: {first}" + + # A fresh loop, exactly as a second asyncio.run() in a test suite creates. + second = asyncio.run(round_trip()) + assert not second, f"errors after switching event loop: {second}" + + third = asyncio.run(round_trip()) + assert not third, f"errors in third loop: {third}" + + +def test_each_event_loop_gets_its_own_semaphore(): + async def grab(): + return get_r_semaphore() + + first = asyncio.run(grab()) + second = asyncio.run(grab()) + assert first is not second, "semaphore must not be shared across loops" + + +@pytest.mark.asyncio +async def test_semaphore_is_stable_within_one_loop(): + assert get_r_semaphore() is get_r_semaphore() + + +@pytest.mark.asyncio +async def test_concurrency_is_actually_capped(): + """The limiter must still limit -- not just avoid raising.""" + limit = get_config().r.max_concurrent + live = 0 + peak = 0 + + async def occupy(): + nonlocal live, peak + async with get_r_semaphore(): + live += 1 + peak = max(peak, live) + await asyncio.sleep(0.01) + live -= 1 + + await asyncio.gather(*[occupy() for _ in range(limit * 3)]) + assert peak == limit, f"expected peak {limit}, got {peak}" diff --git a/tests/integration/tools/test_r_error_handling.py b/tests/integration/tools/test_r_error_handling.py index 77f76cd..f6e7cd7 100644 --- a/tests/integration/tools/test_r_error_handling.py +++ b/tests/integration/tools/test_r_error_handling.py @@ -16,6 +16,7 @@ import pytest from rmcp.core.context import Context, LifespanState +from rmcp.r_integration import RExecutionError from rmcp.tools.helpers import suggest_fix, validate_data from rmcp.tools.regression import ( linear_model, @@ -102,15 +103,19 @@ async def test_logistic_regression_separation_warning(self, context): {"data": data, "formula": "outcome ~ predictor", "family": "binomial"}, ) - # Should not fail, but should contain warning information - assert "model_summary" in result + # Perfect separation still fits, but the fit is degenerate: R + # inflates the standard errors, which drives p-values toward 1. + assert "coefficients" in result + assert "std_errors" in result + assert result["std_errors"], "separation should still report std errors" + assert max(abs(v) for v in result["std_errors"].values()) > 100, ( + "perfect separation should produce inflated standard errors" + ) - # Check if warning information is captured - # (R warnings should be included in output or handled gracefully) print("✅ Logistic regression handled separation scenario") - print(f" Model converged: {result.get('converged', 'unknown')}") + print(f" Max std error: {max(result['std_errors'].values()):.1f}") - except Exception as e: + except RExecutionError as e: # If it fails, the error should be informative error_msg = str(e) assert len(error_msg) > 10, "Error message should be descriptive" @@ -136,7 +141,7 @@ async def test_linear_regression_collinearity_warning(self, context): print(f" R-squared: {result.get('r_squared', 'unknown')}") print(f" DF residual: {result.get('df_residual', 'unknown')}") - except Exception as e: + except RExecutionError as e: error_msg = str(e) assert "rank" in error_msg.lower() or "collinearity" in error_msg.lower() print(f"✅ Collinearity error properly identified: {error_msg[:100]}...") @@ -159,7 +164,7 @@ async def test_chi_square_small_sample_warning(self, context): print(f" Test statistic: {result.get('test_statistic', 'unknown')}") print(f" P-value: {result.get('p_value', 'unknown')}") - except Exception as e: + except RExecutionError as e: error_msg = str(e) # Error could be about missing variables OR statistical issues error_lower = error_msg.lower() @@ -192,7 +197,7 @@ async def test_missing_package_error(self, context): assert "aic" in result or "coefficients" in result print("✅ ARIMA model succeeded (forecast package available)") - except Exception as e: + except RExecutionError as e: error_msg = str(e) if "package" in error_msg.lower(): assert "forecast" in error_msg or "there is no package" in error_msg @@ -221,7 +226,7 @@ async def test_data_type_error(self, context): # If it does, R coerced the data somehow print("⚠️ Linear model handled mixed data types (R coercion occurred)") - except Exception as e: + except RExecutionError as e: error_msg = str(e) # Should contain information about data type issues assert any( @@ -242,7 +247,7 @@ async def test_insufficient_data_error(self, context): # Should not succeed with just one data point print("⚠️ Linear model handled minimal data (unexpected)") - except Exception as e: + except RExecutionError as e: error_msg = str(e) # Should contain information about insufficient data assert any( @@ -270,7 +275,7 @@ async def test_file_not_found_error(self, context): # Should not succeed print("⚠️ File read succeeded for nonexistent file (unexpected)") - except Exception as e: + except RExecutionError as e: error_msg = str(e) # Should contain file-related error information assert any( @@ -392,7 +397,7 @@ async def test_error_messages_are_user_friendly(self, context): try: await scenario["tool"](context, scenario["args"]) print(f"⚠️ {scenario['description']} didn't trigger expected error") - except Exception as e: + except RExecutionError as e: error_msg = str(e) # Error message quality checks @@ -438,7 +443,7 @@ async def test_error_context_preservation(self, context): await linear_model( context, {"data": problematic_data, "formula": "sales ~ marketing"} ) - except Exception as e: + except RExecutionError as e: error_msg = str(e) # Error should preserve context about what went wrong diff --git a/tests/integration/transport/test_http_transport_integration.py b/tests/integration/transport/test_http_transport_integration.py index 75f9b0f..632c452 100644 --- a/tests/integration/transport/test_http_transport_integration.py +++ b/tests/integration/transport/test_http_transport_integration.py @@ -69,7 +69,8 @@ async def test_tools_list_request(self, full_app): assert {"linear_model", "load_example", "summary_stats"} <= names linear_model = next(t for t in tools if t["name"] == "linear_model") assert "inputSchema" in linear_model - assert "outputSchema" in linear_model + # outputSchema is intentionally kept off the wire; see ToolsRegistry.list_tools + assert "outputSchema" not in linear_model @pytest.mark.local async def test_tool_call_request(self, full_app): diff --git a/tests/scenarios/test_deployment_scenarios.py b/tests/scenarios/test_deployment_scenarios.py index 1b6490a..61dea85 100644 --- a/tests/scenarios/test_deployment_scenarios.py +++ b/tests/scenarios/test_deployment_scenarios.py @@ -463,7 +463,7 @@ def test_docker_complete_analysis_workflow(self, production_docker_image): finally: try: Path(temp_file).unlink() - except: + except OSError: pass def test_docker_performance_benchmarks(self, production_docker_image): diff --git a/tests/scenarios/test_excel_plotting_scenarios.py b/tests/scenarios/test_excel_plotting_scenarios.py index c5e41df..9bf5b55 100644 --- a/tests/scenarios/test_excel_plotting_scenarios.py +++ b/tests/scenarios/test_excel_plotting_scenarios.py @@ -154,27 +154,21 @@ async def test_other_problematic_tools(): ("linear_model", {"data": test_data, "formula": "y ~ x"}), ("t_test", {"data": test_data, "variable": "y"}), ] - all_passed = True + failures = [] for tool_name, args in tests: - print(f"🧪 Testing {tool_name}...", end=" ") request = { "jsonrpc": "2.0", "id": 1, "method": "tools/call", "params": {"name": tool_name, "arguments": args}, } - try: - response = await server.handle_request(request) - if "result" in response: - print("✅") - else: - error = response.get("error", {}) - print(f"❌ ({error.get('message', 'Unknown error')})") - all_passed = False - except Exception as e: - print(f"💥 (Exception: {e})") - all_passed = False - return all_passed + response = await server.handle_request(request) + if "result" not in response: + failures.append((tool_name, response.get("error", "no result key"))) + elif response["result"].get("isError"): + failures.append((tool_name, response["result"]["content"])) + + assert not failures, f"tools failed: {failures}" async def main(): diff --git a/tests/scenarios/test_file_output_scenarios.py b/tests/scenarios/test_file_output_scenarios.py index ca922db..8076e99 100644 --- a/tests/scenarios/test_file_output_scenarios.py +++ b/tests/scenarios/test_file_output_scenarios.py @@ -8,7 +8,8 @@ import pytest from rmcp.core.context import Context, LifespanState -from rmcp.security.vfs import VFS +from rmcp.security.vfs import VFS, VFSError +from rmcp.tools.fileops import write_csv from rmcp.tools.flexible_r import approve_operation, execute_r_analysis @@ -28,10 +29,6 @@ async def mock_context_with_vfs(): context = Context.create("test", "test", lifespan) context.temp_dir = temp_dir - # Initialize approval state for the context if not already present - if not hasattr(context, "_approved_operations"): - context._approved_operations = {} - # Copy test iris data to temp directory to eliminate network dependency import shutil @@ -347,50 +344,83 @@ async def test_multiple_file_formats(self, mock_context_with_vfs): assert txt_path.exists() and txt_path.stat().st_size > 0 @pytest.mark.asyncio - async def test_vfs_security_boundaries(self, mock_context_with_vfs): - """Test that VFS security boundaries are maintained.""" + async def test_write_tool_refuses_path_outside_allowed_roots( + self, mock_context_with_vfs + ): + """write_csv must refuse a destination outside the VFS roots. + + The check has to happen before R starts: the subprocess writes the + bytes, so this is the last point Python controls the destination. + """ context = mock_context_with_vfs + outside = Path(tempfile.mkdtemp()) / "unauthorized.csv" - # Approve file operations - await approve_operation( + with pytest.raises(VFSError): + await write_csv( + context, + {"data": {"x": [1, 2, 3]}, "file_path": str(outside)}, + ) + + assert not outside.exists(), "refused write still created the file" + + @pytest.mark.asyncio + async def test_write_tool_allows_path_inside_allowed_roots( + self, mock_context_with_vfs + ): + """The confinement must not block legitimate writes.""" + context = mock_context_with_vfs + inside = Path(context.temp_dir) / "authorized.csv" + + result = await write_csv( context, - { - "operation_type": "file_operations", - "specific_operation": "write.csv", - "action": "approve", - "directory": context.temp_dir, - }, + {"data": {"x": [1, 2, 3]}, "file_path": str(inside)}, ) - # Try to write outside allowed directory - should fail - outside_dir_code = """ - data <- data.frame(x = 1:5, y = 1:5) - write.csv(data, "/tmp/unauthorized.csv", row.names = FALSE) - result <- list(written = TRUE) + assert result["success"] is True + assert inside.exists() + + @pytest.mark.asyncio + async def test_write_confinement_is_off_without_a_vfs(self): + """No VFS configured means no policy to enforce, so writes proceed. + + This keeps programmatic embedders working. Both CLI entry points call + MCPServer.configure(), so real servers always have a VFS. """ + context = Context.create("test", "test", LifespanState()) + target = Path(tempfile.mkdtemp()) / "no_vfs.csv" - result = await execute_r_analysis( + result = await write_csv( context, - { - "r_code": outside_dir_code, - "description": "Test writing outside allowed directory", - }, + {"data": {"x": [1, 2, 3]}, "file_path": str(target)}, ) - # The test should either: - # 1. Fail during R execution due to approval system blocking, OR - # 2. Succeed in R but VFS should prevent actual file creation - unauthorized_file = Path("/tmp/unauthorized.csv") + assert result["success"] is True + assert target.exists() - # Clean up any existing file first - if unauthorized_file.exists(): - unauthorized_file.unlink() + @pytest.mark.asyncio + async def test_unapproved_r_write_is_blocked_by_approval( + self, mock_context_with_vfs + ): + """execute_r_analysis gates writes on approval, not on the VFS. + + Note the limit deliberately asserted here: approval is a regex check on + the submitted R source. Once R runs it writes wherever it likes -- the + VFS does not confine the subprocess. Do not read this test as proof + that unapproved paths are unreachable. + """ + context = mock_context_with_vfs - # After R execution, the unauthorized file should not exist - assert not unauthorized_file.exists(), ( - "VFS security was bypassed - unauthorized file was created" + result = await execute_r_analysis( + context, + { + "r_code": ( + "data <- data.frame(x = 1:5)\n" + 'write.csv(data, "unauthorized.csv", row.names = FALSE)\n' + "result <- list(written = TRUE)" + ), + "description": "Write without prior approval", + }, ) - # The operation should have been blocked by approval system - if not result["success"]: - assert "OPERATION_APPROVAL_NEEDED" in result.get("error", "") + assert result["success"] is False + assert "OPERATION_APPROVAL_NEEDED" in str(result.get("error", "")) diff --git a/tests/scenarios/test_realistic_scenarios.py b/tests/scenarios/test_realistic_scenarios.py index 39033ca..dbfb427 100644 --- a/tests/scenarios/test_realistic_scenarios.py +++ b/tests/scenarios/test_realistic_scenarios.py @@ -8,8 +8,6 @@ Each test uses the new MCP architecture with proper tools. """ -import asyncio - import pytest # Add rmcp to path @@ -74,30 +72,25 @@ async def test_business_analyst_scenario(): 2024, ], } - try: - context = await create_test_context() - # Question: "How much does marketing spend affect sales?" - result = await linear_model( - context, {"data": sales_data, "formula": "sales ~ marketing + quarter"} - ) - # Business validation - r_squared = result["r_squared"] - marketing_effect = result["coefficients"]["marketing"] - marketing_pvalue = result["p_values"]["marketing"] - print("✅ Sales Model Results:") - print(f" 📈 Marketing ROI: ${marketing_effect:.2f} sales per $1 marketing") - print(f" 📊 Model explains {r_squared:.1%} of sales variation") - print(f" 🎯 Marketing effect p-value: {marketing_pvalue:.4f}") - print(f" 📋 Sample size: {result['n_obs']} quarters") - # Business success criteria - assert marketing_effect > 0, "Marketing should increase sales" - assert marketing_pvalue < 0.05, "Marketing effect should be significant" - assert r_squared > 0.8, "Model should explain >80% of variance" - print("✅ PASS: Business analyst can predict sales effectively") - return True - except Exception as e: - print(f"❌ FAIL: Business scenario error - {e}") - return False + context = await create_test_context() + # Question: "How much does marketing spend affect sales?" + result = await linear_model( + context, {"data": sales_data, "formula": "sales ~ marketing + quarter"} + ) + # Business validation + r_squared = result["r_squared"] + marketing_effect = result["coefficients"]["marketing"] + marketing_pvalue = result["p_values"]["marketing"] + print("✅ Sales Model Results:") + print(f" 📈 Marketing ROI: ${marketing_effect:.2f} sales per $1 marketing") + print(f" 📊 Model explains {r_squared:.1%} of sales variation") + print(f" 🎯 Marketing effect p-value: {marketing_pvalue:.4f}") + print(f" 📋 Sample size: {result['n_obs']} quarters") + # Business success criteria + assert marketing_effect > 0, "Marketing should increase sales" + assert marketing_pvalue < 0.05, "Marketing effect should be significant" + assert r_squared > 0.8, "Model should explain >80% of variance" + print("✅ PASS: Business analyst can predict sales effectively") @pytest.mark.asyncio @@ -125,44 +118,39 @@ async def test_economist_scenario(): "Q4-23", ], } - try: - context = await create_test_context() - # Question: "What are the relationships between key macro variables?" - corr_result = await correlation_analysis( - context, - { - "data": macro_data, - "variables": ["gdp_growth", "inflation", "unemployment"], - "method": "pearson", - }, - ) - # Economic theory validation - corr_matrix = corr_result["correlation_matrix"] - # Okun's Law: GDP growth and unemployment should be negatively correlated - gdp_unemp_corr = corr_matrix["gdp_growth"][ - "unemployment" - ] # Access by variable name - print("✅ Macroeconomic Correlations:") - print(f" 📉 GDP-Unemployment: {gdp_unemp_corr:.3f} (Okun's Law)") - print(f" 📊 Sample size: {corr_result['n_obs']} observations") - print(f" 🔬 Variables analyzed: {', '.join(corr_result['variables'])}") - # Test Phillips Curve: inflation ~ unemployment - phillips_result = await linear_model( - context, {"data": macro_data, "formula": "inflation ~ unemployment"} - ) - phillips_coef = phillips_result["coefficients"]["unemployment"] - phillips_r2 = phillips_result["r_squared"] - print(f" 📈 Phillips Curve slope: {phillips_coef:.3f}") - print(f" 📊 Phillips R²: {phillips_r2:.3f}") - # Economic validation - assert gdp_unemp_corr < 0, ( - "GDP growth and unemployment should be negatively correlated" - ) - print("✅ PASS: Economist can analyze macroeconomic relationships") - return True - except Exception as e: - print(f"❌ FAIL: Economist scenario error - {e}") - return False + context = await create_test_context() + # Question: "What are the relationships between key macro variables?" + corr_result = await correlation_analysis( + context, + { + "data": macro_data, + "variables": ["gdp_growth", "inflation", "unemployment"], + "method": "pearson", + }, + ) + # Economic theory validation + corr_matrix = corr_result["correlation_matrix"] + # Okun's Law: GDP growth and unemployment should be negatively correlated + gdp_unemp_corr = corr_matrix["gdp_growth"][ + "unemployment" + ] # Access by variable name + print("✅ Macroeconomic Correlations:") + print(f" 📉 GDP-Unemployment: {gdp_unemp_corr:.3f} (Okun's Law)") + print(f" 📊 Sample size: {corr_result['n_obs']} observations") + print(f" 🔬 Variables analyzed: {', '.join(corr_result['variables'])}") + # Test Phillips Curve: inflation ~ unemployment + phillips_result = await linear_model( + context, {"data": macro_data, "formula": "inflation ~ unemployment"} + ) + phillips_coef = phillips_result["coefficients"]["unemployment"] + phillips_r2 = phillips_result["r_squared"] + print(f" 📈 Phillips Curve slope: {phillips_coef:.3f}") + print(f" 📊 Phillips R²: {phillips_r2:.3f}") + # Economic validation + assert gdp_unemp_corr < 0, ( + "GDP growth and unemployment should be negatively correlated" + ) + print("✅ PASS: Economist can analyze macroeconomic relationships") @pytest.mark.asyncio @@ -219,40 +207,33 @@ async def test_data_scientist_scenario(): ], "support_tickets": [0, 3, 1, 5, 0, 1, 4, 6, 0, 4, 1, 5, 0, 3, 0, 2, 4, 7, 0, 3], } - try: - context = await create_test_context() - # Question: "Can I predict which customers will churn?" - churn_model = await logistic_regression( - context, - { - "data": churn_data, - "formula": "churn ~ tenure_months + monthly_charges + support_tickets", - "family": "binomial", - "link": "logit", - }, - ) - # Model performance validation - accuracy = churn_model.get("accuracy", 0) - mcfadden_r2 = churn_model.get("mcfadden_r_squared", 0) - tenure_coef = churn_model["coefficients"]["tenure_months"] - support_coef = churn_model["coefficients"]["support_tickets"] - print("✅ Churn Prediction Model:") - print(f" 🎯 Accuracy: {accuracy:.1%}") - print(f" 📊 McFadden R²: {mcfadden_r2:.3f}") - print(f" 📉 Tenure effect: {tenure_coef:.4f} (longer tenure = less churn)") - print(f" 📞 Support tickets effect: {support_coef:.4f}") - print(f" 📋 Sample size: {churn_model['n_obs']} customers") - # Data science validation - assert accuracy > 0.6, "Model should achieve >60% accuracy" - assert tenure_coef < 0, "Longer tenure should reduce churn probability" - assert support_coef > 0, ( - "More support tickets should increase churn probability" - ) - print("✅ PASS: Data scientist can build churn prediction model") - return True - except Exception as e: - print(f"❌ FAIL: Data science scenario error - {e}") - return False + context = await create_test_context() + # Question: "Can I predict which customers will churn?" + churn_model = await logistic_regression( + context, + { + "data": churn_data, + "formula": "churn ~ tenure_months + monthly_charges + support_tickets", + "family": "binomial", + "link": "logit", + }, + ) + # Model performance validation + accuracy = churn_model.get("accuracy", 0) + mcfadden_r2 = churn_model.get("mcfadden_r_squared", 0) + tenure_coef = churn_model["coefficients"]["tenure_months"] + support_coef = churn_model["coefficients"]["support_tickets"] + print("✅ Churn Prediction Model:") + print(f" 🎯 Accuracy: {accuracy:.1%}") + print(f" 📊 McFadden R²: {mcfadden_r2:.3f}") + print(f" 📉 Tenure effect: {tenure_coef:.4f} (longer tenure = less churn)") + print(f" 📞 Support tickets effect: {support_coef:.4f}") + print(f" 📋 Sample size: {churn_model['n_obs']} customers") + # Data science validation + assert accuracy > 0.6, "Model should achieve >60% accuracy" + assert tenure_coef < 0, "Longer tenure should reduce churn probability" + assert support_coef > 0, "More support tickets should increase churn probability" + print("✅ PASS: Data scientist can build churn prediction model") @pytest.mark.asyncio @@ -301,80 +282,31 @@ async def test_researcher_scenario(): 4.6, ], } - try: - context = await create_test_context() - # Question: "What is the treatment effect controlling for covariates?" - treatment_model = await linear_model( - context, - { - "data": experiment_data, - "formula": "outcome ~ treatment + age + baseline_score", - }, - ) - # Research validation - treatment_coef = treatment_model["coefficients"]["treatment"] - treatment_pvalue = treatment_model["p_values"]["treatment"] - baseline_coef = treatment_model["coefficients"]["baseline_score"] - r_squared = treatment_model["r_squared"] - print("✅ Treatment Effect Results:") - print(f" 🧪 Treatment effect: {treatment_coef:.3f} points") - print(f" 📊 Significance: p = {treatment_pvalue:.4f}") - print(f" 📈 Baseline control: {baseline_coef:.3f}") - print(f" 🎯 Model R²: {r_squared:.3f}") - print(f" 👥 Sample size: {treatment_model['n_obs']} participants") - # Research standards - assert treatment_coef > 0, "Treatment should have positive effect" - assert treatment_pvalue < 0.05, ( - "Treatment effect should be statistically significant" - ) - assert baseline_coef > 0, "Baseline should predict outcome" - assert r_squared > 0.7, "Model should have good explanatory power" - print("✅ PASS: Researcher can analyze treatment effects") - return True - except Exception as e: - print(f"❌ FAIL: Research scenario error - {e}") - return False - - -async def run_all_scenarios(): - """Run all realistic user scenarios.""" - print("🎯 RMCP MCP Server - Realistic User Testing") - print("=" * 55) - print("Testing what real users want to accomplish with R analysis\n") - scenarios = [ - ("Business Analyst", test_business_analyst_scenario), - ("Economist", test_economist_scenario), - ("Data Scientist", test_data_scientist_scenario), - ("Academic Researcher", test_researcher_scenario), - ] - results = [] - for scenario_name, test_func in scenarios: - try: - success = await test_func() - results.append((scenario_name, success)) - except Exception as e: - print(f"❌ {scenario_name} scenario crashed: {e}") - results.append((scenario_name, False)) - # Summary - print("\n" + "=" * 55) - print("🎯 REALISTIC SCENARIO RESULTS") - print("=" * 55) - passed = sum(1 for _, success in results if success) - total = len(results) - for scenario_name, success in results: - status = "✅ PASS" if success else "❌ FAIL" - print(f"{status} {scenario_name}") - print(f"\n📊 Overall Success Rate: {passed}/{total} ({passed / total:.1%})") - if passed == total: - print("🎉 ALL SCENARIOS PASSED!") - print("✅ Users can accomplish their real-world R analysis goals") - print("🚀 RMCP is ready for production use!") - else: - print("⚠️ SOME SCENARIOS FAILED") - print("🔧 Need to fix issues before production deployment") - return passed == total - - -if __name__ == "__main__": - success = asyncio.run(run_all_scenarios()) - exit(0 if success else 1) + context = await create_test_context() + # Question: "What is the treatment effect controlling for covariates?" + treatment_model = await linear_model( + context, + { + "data": experiment_data, + "formula": "outcome ~ treatment + age + baseline_score", + }, + ) + # Research validation + treatment_coef = treatment_model["coefficients"]["treatment"] + treatment_pvalue = treatment_model["p_values"]["treatment"] + baseline_coef = treatment_model["coefficients"]["baseline_score"] + r_squared = treatment_model["r_squared"] + print("✅ Treatment Effect Results:") + print(f" 🧪 Treatment effect: {treatment_coef:.3f} points") + print(f" 📊 Significance: p = {treatment_pvalue:.4f}") + print(f" 📈 Baseline control: {baseline_coef:.3f}") + print(f" 🎯 Model R²: {r_squared:.3f}") + print(f" 👥 Sample size: {treatment_model['n_obs']} participants") + # Research standards + assert treatment_coef > 0, "Treatment should have positive effect" + assert treatment_pvalue < 0.05, ( + "Treatment effect should be statistically significant" + ) + assert baseline_coef > 0, "Baseline should predict outcome" + assert r_squared > 0.7, "Model should have good explanatory power" + print("✅ PASS: Researcher can analyze treatment effects") diff --git a/tests/unit/core/test_large_data_store.py b/tests/unit/core/test_large_data_store.py new file mode 100644 index 0000000..1c28cb2 --- /dev/null +++ b/tests/unit/core/test_large_data_store.py @@ -0,0 +1,89 @@ +#!/usr/bin/env python3 +"""Bounds on the oversized-result store. + +Results too large to inline are parked behind an ``rmcp://data/{id}`` link. +Each is at least 50KB, so the store has to evict; an unbounded one grows for +the life of the process, which matters for the long-running HTTP server. +""" + +from urllib.parse import urlparse + +import pytest +from rmcp.core.context import Context, LifespanState +from rmcp.registries.resources import ResourcesRegistry +from rmcp.registries.tools import MAX_STORED_RESULTS, ToolsRegistry + + +def _oversized_payload(): + """Comfortably past both the 50KB and 1000-row thresholds.""" + return {"col_a": list(range(20000)), "col_b": list(range(20000))} + + +def _store_one(registry): + uri = registry._check_for_large_data_and_create_resource(_oversized_payload()) + assert uri is not None, "payload should have been diverted to a resource link" + return uri + + +class TestLargeDataStoreEviction: + def test_store_stays_bounded(self): + registry = ToolsRegistry() + for _ in range(MAX_STORED_RESULTS + 10): + _store_one(registry) + + assert len(registry._large_data_store) == MAX_STORED_RESULTS + + def test_oldest_entries_are_evicted_first(self): + registry = ToolsRegistry() + uris = [_store_one(registry) for _ in range(MAX_STORED_RESULTS + 5)] + + oldest = uris[0].rsplit("/", 1)[1] + newest = uris[-1].rsplit("/", 1)[1] + + assert oldest not in registry._large_data_store + assert newest in registry._large_data_store + + def test_small_payloads_are_not_stored(self): + registry = ToolsRegistry() + assert ( + registry._check_for_large_data_and_create_resource({"x": [1, 2, 3]}) is None + ) + assert len(registry._large_data_store) == 0 + + +class TestExpiredResourceLink: + @pytest.mark.asyncio + async def test_evicted_link_explains_itself(self): + """An expired link must say it expired, not look like a bad URI.""" + registry = ToolsRegistry() + uris = [_store_one(registry) for _ in range(MAX_STORED_RESULTS + 5)] + + class _Server: + tools = registry + + context = Context.create("test", "test", LifespanState()) + + with pytest.raises(ValueError) as excinfo: + await ResourcesRegistry()._read_stored_rmcp_data( + context, _Server(), urlparse(uris[0]) + ) + + message = str(excinfo.value) + assert "no longer available" in message + assert str(MAX_STORED_RESULTS) in message + + @pytest.mark.asyncio + async def test_live_link_still_resolves(self): + registry = ToolsRegistry() + uri = _store_one(registry) + + class _Server: + tools = registry + + context = Context.create("test", "test", LifespanState()) + result = await ResourcesRegistry()._read_stored_rmcp_data( + context, _Server(), urlparse(uri) + ) + + assert result["contents"][0]["mimeType"] == "application/json" + assert "col_a" in result["contents"][0]["text"] diff --git a/tests/unit/core/test_sdk_adapter.py b/tests/unit/core/test_sdk_adapter.py index 82f796e..de35d4c 100644 --- a/tests/unit/core/test_sdk_adapter.py +++ b/tests/unit/core/test_sdk_adapter.py @@ -82,11 +82,13 @@ async def test_list_and_call_tool(rmcp_server): names = [tool.name for tool in tools.tools] assert "echo" in names echo = next(tool for tool in tools.tools if tool.name == "echo") - assert echo.outputSchema is not None + # outputSchema is not advertised: it dominated the tools/list payload + # without telling the model anything the first result doesn't. + assert echo.outputSchema is None result = await session.call_tool("echo", {"message": "hello"}) assert result.isError is not True - # structuredContent is the raw payload and conforms to outputSchema + # ...but results are still validated against it server-side assert result.structuredContent == {"echoed": "hello", "length": 5} diff --git a/tests/unit/tools/test_all_tool_schemas.py b/tests/unit/tools/test_all_tool_schemas.py index 7947ddd..d0a607d 100644 --- a/tests/unit/tools/test_all_tool_schemas.py +++ b/tests/unit/tools/test_all_tool_schemas.py @@ -223,21 +223,20 @@ def test_additional_properties_handling(self): tool = statistical_tests.t_test schema = tool._mcp_tool_input_schema - # Valid input with extra property + # Otherwise-valid input, plus one property the schema does not declare input_with_extra = { "data": {"x": [1, 2, 3]}, - "variables": ["x"], + "variable": "x", "extra_property": "should_be_ignored_or_rejected", } - # Schema validation behavior depends on additionalProperties setting - try: - validate(instance=input_with_extra, schema=schema) - # If validation passes, additionalProperties is allowed - assert True - except ValidationError: - # If validation fails, additionalProperties is not allowed - assert True + # Input schemas are deliberately permissive about unknown properties + # (output schemas are the strict ones). Assert that, rather than + # accepting either outcome. + assert schema.get("additionalProperties") is not False, ( + "input schemas should tolerate unknown properties" + ) + validate(instance=input_with_extra, schema=schema) if __name__ == "__main__": diff --git a/tests/unit/tools/test_flexible_r.py b/tests/unit/tools/test_flexible_r.py index b2cbc94..d24c038 100644 --- a/tests/unit/tools/test_flexible_r.py +++ b/tests/unit/tools/test_flexible_r.py @@ -6,7 +6,12 @@ import pytest from jsonschema import ValidationError, validate -from rmcp.tools.flexible_r import execute_r_analysis, list_allowed_r_packages +from rmcp.core.context import Context, LifespanState +from rmcp.tools.flexible_r import ( + execute_r_analysis, + list_allowed_r_packages, + validate_r_code, +) class TestFlexibleRSchemaValidation: @@ -96,5 +101,69 @@ def test_list_allowed_packages_schema(self): pass +class TestDeclaredPackagesAreValidated: + """The `packages` argument is concatenated into library(...) lines. + + Unlike names parsed out of r_code, these are never seen by the pattern + scans, so they must be validated on the way in or they bypass the + allowlist, the dangerous-pattern block, and every approval category. + """ + + def _context(self): + return Context.create("test", "test", LifespanState()) + + def test_injection_through_packages_is_refused(self): + is_safe, error = validate_r_code( + "result <- 1", + self._context(), + packages=["stats); RMCP_INJECTION_MARKER <- TRUE; library(utils"], + ) + assert not is_safe + assert "Invalid package name" in error + + @pytest.mark.parametrize( + "name", + [ + "stats)", + "utils; system('id')", + "pkg name", + "", + "2pkg", + "../../etc/passwd", + ], + ) + def test_non_identifier_package_names_are_refused(self, name): + is_safe, error = validate_r_code( + "result <- 1", self._context(), packages=[name] + ) + assert not is_safe + assert "Invalid package name" in error + + def test_package_outside_allowlist_requests_approval(self): + is_safe, error = validate_r_code( + "result <- 1", self._context(), packages=["definitelyNotOnTheAllowlist"] + ) + assert not is_safe + assert error == "APPROVAL_NEEDED:definitelyNotOnTheAllowlist" + + def test_session_approved_package_is_accepted(self): + context = self._context() + context.lifespan.approved_packages.add("notarealpkg") + is_safe, error = validate_r_code( + "result <- 1", context, packages=["notarealpkg"] + ) + assert is_safe, error + + def test_allowlisted_package_is_accepted(self): + is_safe, error = validate_r_code( + "result <- 1", self._context(), packages=["dplyr", "ggplot2"] + ) + assert is_safe, error + + def test_omitting_packages_is_unaffected(self): + is_safe, error = validate_r_code("result <- 1", self._context()) + assert is_safe, error + + if __name__ == "__main__": pytest.main([__file__, "-v"]) diff --git a/tests/unit/tools/test_operation_approval.py b/tests/unit/tools/test_operation_approval.py index 8907191..bf3e94f 100644 --- a/tests/unit/tools/test_operation_approval.py +++ b/tests/unit/tools/test_operation_approval.py @@ -8,6 +8,7 @@ from rmcp.tools.flexible_r import ( OPERATION_CATEGORIES, approve_operation, + approve_r_package, is_operation_approved, validate_r_code, ) @@ -89,10 +90,10 @@ async def test_session_approval_tracking(self, mock_context): }, ) - # Check that approval is tracked - assert hasattr(mock_context, "_approved_operations") - assert "file_operations" in mock_context._approved_operations - assert "ggsave" in mock_context._approved_operations["file_operations"] + # Approval is tracked on the lifespan, so it outlives this request + approvals = mock_context.lifespan.approved_operations + assert "file_operations" in approvals + assert "ggsave" in approvals["file_operations"] def test_is_operation_approved_logic(self, mock_context): """Test the operation approval checking logic.""" @@ -100,7 +101,7 @@ def test_is_operation_approved_logic(self, mock_context): assert not is_operation_approved(mock_context, "file_operations", "ggsave") # Add approval manually - mock_context._approved_operations = { + mock_context.lifespan.approved_operations = { "file_operations": {"ggsave": {"approved_at": 123456}} } @@ -133,7 +134,7 @@ def test_validation_requires_approval_for_install_packages(self): def test_validation_allows_approved_operations(self, mock_context): """Test that approved operations pass validation.""" # Add approval - mock_context._approved_operations = { + mock_context.lifespan.approved_operations = { "file_operations": {"ggsave": {"approved_at": 123456}} } @@ -195,3 +196,64 @@ def test_system_operations_patterns(self): assert any("system" in pattern for pattern in patterns) assert any("shell" in pattern for pattern in patterns) assert any("Sys\\.setenv" in pattern for pattern in patterns) + + +class TestApprovalPersistsAcrossRequests: + """Approvals must outlive the Context they were granted in. + + A fresh Context is built per MCP request, so approval state stored on the + Context is forgotten before the next tools/call arrives. + """ + + @pytest.mark.asyncio + async def test_operation_approval_survives_new_context(self): + lifespan = LifespanState() + approving_context = Context.create("req-1", "approve_operation", lifespan) + + await approve_operation( + approving_context, + { + "operation_type": "file_operations", + "specific_operation": "ggsave", + "action": "approve", + }, + ) + + # A later request gets a brand-new Context over the same lifespan + later_context = Context.create("req-2", "execute_r_analysis", lifespan) + + assert is_operation_approved(later_context, "file_operations", "ggsave") + is_safe, error = validate_r_code( + 'ggsave("plot.png", plot = p)', context=later_context + ) + assert is_safe, error + + @pytest.mark.asyncio + async def test_package_approval_survives_new_context(self): + lifespan = LifespanState() + approving_context = Context.create("req-1", "approve_r_package", lifespan) + + await approve_r_package( + approving_context, + {"package_name": "notarealpkg", "action": "approve"}, + ) + + later_context = Context.create("req-2", "execute_r_analysis", lifespan) + is_safe, error = validate_r_code("library(notarealpkg)", context=later_context) + assert is_safe, error + + @pytest.mark.asyncio + async def test_separate_lifespans_do_not_share_approvals(self): + lifespan_a = LifespanState() + await approve_operation( + Context.create("req-1", "approve_operation", lifespan_a), + { + "operation_type": "file_operations", + "specific_operation": "ggsave", + "action": "approve", + }, + ) + + lifespan_b = LifespanState() + other = Context.create("req-1", "execute_r_analysis", lifespan_b) + assert not is_operation_approved(other, "file_operations", "ggsave") diff --git a/uv.lock b/uv.lock index 6970f5d..5ec7f06 100644 --- a/uv.lock +++ b/uv.lock @@ -1384,7 +1384,7 @@ wheels = [ [[package]] name = "rmcp" -version = "0.9.1" +version = "0.10.0" source = { editable = "." } dependencies = [ { name = "click" },