diff --git a/CHANGELOG.md b/CHANGELOG.md index c01cffd..cca3e9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,39 @@ All notable changes to this project are documented here. Format follows ## [Unreleased] +## [0.3.1] - 2026-08-02 + +### Added +- Every tool now declares what it does to the machine: `title`, and + `ToolAnnotations` with `readOnlyHint`, `destructiveHint`, `idempotentHint` + and `openWorldHint`. Rigout advertised fifteen tools identically, so reading a + CPU count and running an arbitrary command as root reached a client looking + the same and anything wanting to warn before the second had only the name to + go on. Four tools are read-only, nine are destructive, and two are neither - + they change something but only add or open it, never overwrite or remove. + Anything that runs a caller's command is marked destructive and not + idempotent, because what it does is decided by the caller and cannot be + known here. + +### Changed +- The `mcp` bound widens to `>=1.0.0,<3`: Rigout now runs on both majors. 1.x + registers the tool handlers with decorators and 2.x removed them for explicit + `add_request_handler`, and that is the entire incompatibility - every tool + definition constructs unchanged, because 2.x renamed `Tool`'s fields while + keeping the camelCase spellings as aliases. Two reads did need care, since + the aliases cover construction and not attribute access: `CallToolResult`'s + error flag is `is_error` on 2.x and `isError` on 1.x, and the same holds for + annotation hints. Both are read through helpers that work either way. + Supporting both is deliberate rather than transitional - nobody is pushed onto + a major the week it appears, and nobody is stranded on the old one. +- `VERSIONING.md` records which capabilities of the current MCP line Rigout + adopts and which it declines. Tasks - long-running work a client polls rather + than waits for - are declined despite addressing Rigout's oldest limitation, + because the API is deprecated in the version Rigout pins and + `mcp.server.experimental.task_support` raises `ModuleNotFoundError` on 2.0.0, + the version Rigout must move to next. The types survive there, which makes the + feature look available to anyone reading `mcp.types`. + ## [0.3.0] - 2026-08-01 ### Added diff --git a/VERSIONING.md b/VERSIONING.md index c2c87ec..1291002 100644 --- a/VERSIONING.md +++ b/VERSIONING.md @@ -79,6 +79,75 @@ Two things watch this so it does not become permanent: - The same workflow reports which dependencies are held back and by how far, so an overdue major is visible rather than forgotten. +## What Rigout adopts from a new MCP, and what it declines + +Being behind on a major does not mean ignoring what the current line offers. Two +capabilities were assessed against mcp 1.29 in August 2026, and they went opposite ways. + +**Tool annotations: adopted.** `Tool.title` and `ToolAnnotations` - `readOnlyHint`, +`destructiveHint`, `idempotentHint`, `openWorldHint` - let a client tell a question apart +from an action before it runs one, which matters more here than in most servers: reading +a CPU count and running an arbitrary command as root are both tools Rigout offers. The +fields are in the specification, present in 1.x and 2.x alike, and additive to clients +that ignore them. + +**Tasks: declined, and this is worth stating so it is not rediscovered.** `Tool.execution` +with `taskSupport`, the `Task` type, `tasks/get` and its siblings, and +`mcp.server.experimental.task_support` together describe long-running work a client polls +rather than waits for. That addresses Rigout's oldest limitation directly - a command +that outlives its timeout fails, and builds, installs and downloads all can. + +It is not adopted, because the API says of itself: + +> The experimental tasks API is deprecated and will be removed in mcp 2.0: tasks +> (SEP-1686) were removed from the MCP specification and are expected to return as a +> separate MCP extension. + +Checked rather than taken on the warning's word: `mcp.server.experimental.task_support` +raises `ModuleNotFoundError` on 2.0.0. The types survive there, which makes the feature +look available to anyone reading `mcp.types`, but the server half is gone. + +Building on it would mean shipping a feature on an interface that is deprecated in the +version Rigout pins and absent from the version it must move to next. That is the same +trade as the unbounded `mcp>=1.0.0` in 0.2.0: it works until someone else's release day. +When tasks return as an extension, this is worth revisiting, and the reason to revisit is +recorded above. + +## The mcp 2.x migration, mapped + +Rigout pins `mcp>=1.0.0,<2` and speaks MCP protocol `2025-11-25` where 2.x speaks +`2026-07-28`. Moving is a release of its own. What it involves is recorded here because +the discovery is most of the work and is easy to redo badly. + +What does **not** change, checked against 2.0.0 rather than assumed: + +- `Tool(name=..., description=..., inputSchema=...)` constructs unchanged. 2.x renamed the + fields to `input_schema` and friends but keeps the camelCase spellings as aliases, so + every tool definition ports as written. +- `mcp.server.stdio`, `mcp.server.streamable_http_manager`, `mcp.server.models` and + `mcp.types` all still import. +- `Server`, `Server.run` and `create_initialization_options` all survive. + +What does change, and is the whole of the migration: + +- `@server.list_tools()` and `@server.call_tool()` are gone. Registration is now + `server.add_request_handler(method, params_type, handler)`, with + `"tools/list"` taking `PaginatedRequestParams` and `"tools/call"` taking + `CallToolRequestParams`. +- The handlers therefore return results directly rather than the bare list and content + the decorators wrapped, which also removes the wrinkle where an error result has to be + raised as `RuntimeError` for the SDK to rebuild it. + +Two decisions to make before starting, neither obvious: + +- **Whether to support both majors or move.** The incompatibility is only the + registration, so a single `hasattr(Server, "list_tools")` branch would let the cap + widen to `<3` and not force anybody onto a major that is days old. The cost is a fork + in the code that has to be tested twice, on both lines. +- **Whether it is time at all.** Nothing Rigout needs is exclusive to 2.x - tasks, which + looked like the reason to move, are gone from both. The protocol revision is the only + gain, and the caps mean nobody is broken meanwhile. + ## Releasing ```bash diff --git a/production_validation.py b/production_validation.py index 1844bc4..77a30f5 100644 --- a/production_validation.py +++ b/production_validation.py @@ -334,12 +334,13 @@ def validate_runtime_contracts() -> list[str]: from rigout import __version__ from rigout.mcp_http_server import create_app from rigout.server import handle_call_tool_result, server + from rigout.tools._results import result_is_error if getattr(server, "version", None) != __version__: issues.append(f"MCP server version is {getattr(server, 'version', None)}, expected {__version__}") unknown_result = asyncio.run(handle_call_tool_result("definitely_unknown_tool", {})) - if not unknown_result.isError: + if not result_is_error(unknown_result): issues.append("Unknown MCP tools are not marked with isError=true") app = create_app(connection_file=None, setup_token="setup-check", auth_token="bearer-check") diff --git a/pyproject.toml b/pyproject.toml index c0db076..0d3726d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "rigout" -version = "0.3.0" +version = "0.3.1" description = "Let AI agents use your computer through MCP." readme = "README.md" requires-python = ">=3.10" @@ -39,7 +39,7 @@ classifiers = [ # which are not shipped, so it moved to the dev extra. cryptography still arrives # transitively via paramiko, which owns the version it needs. dependencies = [ - "mcp>=1.0.0,<2", + "mcp>=1.0.0,<3", "starlette>=0.37.0,<2", "uvicorn>=0.29.0,<1", "paramiko>=3.0.0,<6", diff --git a/src/rigout/server.py b/src/rigout/server.py index 6d4d235..4f8915c 100644 --- a/src/rigout/server.py +++ b/src/rigout/server.py @@ -1,7 +1,8 @@ import asyncio import logging import sys -from collections.abc import Sequence +from collections.abc import Mapping, Sequence +from typing import Any from mcp.server import NotificationOptions, Server from mcp.server.models import InitializationOptions @@ -11,6 +12,7 @@ ContentBlock, TextContent, Tool, + ToolAnnotations, ) from ._version import __version__ @@ -33,7 +35,7 @@ handle_manage_tunnels, handle_system_monitoring, ) -from .tools._results import transport_safe_result +from .tools._results import build_result, result_is_error, transport_safe_result logger = logging.getLogger(__name__) @@ -49,298 +51,408 @@ server = Server("enhanced-hardware-server", version=__version__) +# --- constructing mcp types on either major --------------------------------------------- +# +# 2.x renamed these fields to snake_case and kept the camelCase spellings as construction +# aliases, so every call below runs unchanged on both. The type checker does not see an +# alias, only the field, and it only ever sees the major that happens to be installed - so +# without these the fifteen tool definitions are fifteen errors on 2.x and none on 1.x. +# Same reasoning as the handler registration at the bottom of this file: keep the fork in +# one named place, and keep everything around it checked. + + +def _rename_for_installed_mcp( + kwargs: dict[str, Any], pairs: dict[str, str], fields: Mapping[str, Any] +) -> dict[str, Any]: + """Respell camelCase keys as snake_case when the model no longer declares them.""" + for camel, snake in pairs.items(): + if camel in kwargs and camel not in fields: + kwargs[snake] = kwargs.pop(camel) + return kwargs + + +def _tool(**kwargs: Any) -> Tool: + """Build a Tool. Callers spell the schema `inputSchema`, as both majors accept.""" + return Tool(**_rename_for_installed_mcp(kwargs, {"inputSchema": "input_schema"}, Tool.model_fields)) + + +def _tool_annotations(**kwargs: Any) -> ToolAnnotations: + """Build ToolAnnotations. Callers spell the hints in camelCase.""" + return ToolAnnotations( + **_rename_for_installed_mcp( + kwargs, + { + "readOnlyHint": "read_only_hint", + "destructiveHint": "destructive_hint", + "idempotentHint": "idempotent_hint", + "openWorldHint": "open_world_hint", + }, + ToolAnnotations.model_fields, + ) + ) + + +# What each tool does to the machine, in the terms MCP defines, so a client can tell a +# question apart from an action before it runs one. Rigout spans the whole range - a tool +# that reads a CPU count and a tool that runs arbitrary commands as root are both here - +# and until now it advertised them identically, leaving every client to guess. +# +# The fields mean what the specification says they mean, and the honest reading is the +# conservative one: +# read_only the tool does not change the machine at all +# destructive it may overwrite or remove something, not merely add +# idempotent calling it again with the same arguments changes nothing further +# open_world it reaches something outside this machine: a remote host, a registry +# +# Anything that can run a caller's command is destructive and not idempotent, because +# what it does is decided by the caller and cannot be known here. +TOOL_ANNOTATIONS: dict[str, tuple[str, bool, bool, bool, bool]] = { + # name: (title, read_only, destructive, idempotent, open_world) + "connect_hardware": ("Connect to hardware", False, False, True, True), + "execute_command": ("Run a command", False, True, False, True), + "create_terminal_session": ("Open a terminal session", False, False, False, True), + "execute_in_terminal": ("Run a command in a session", False, True, False, True), + "list_terminal_sessions": ("List terminal sessions", True, False, True, False), + "close_terminal_session": ("Close a terminal session", False, True, True, True), + "get_hardware_info": ("Read hardware information", True, False, True, True), + "get_server_activity": ("Read Rigout's activity", True, False, True, False), + "manage_tunnels": ("Manage SSH endpoints", False, True, False, True), + "install_software": ("Install packages", False, True, False, True), + "file_operations": ("Read and change files", False, True, False, True), + "system_monitoring": ("Read system metrics", True, False, True, True), + "docker_operations": ("Manage Docker", False, True, False, True), + "bulk_file_transfer": ("Transfer files", False, True, False, True), + "environment_setup": ("Prepare a development environment", False, True, False, True), +} + + +def annotate(tools: list[Tool]) -> list[Tool]: + """Attach the declared annotations and title to each tool. + + Applied here rather than written into each definition so that the classification is + one table somebody can read and argue with, instead of fifteen scattered arguments + where a missing one is invisible. A tool with no entry keeps none, and the metadata + test fails, which is what stops a new tool shipping unclassified. + """ + for tool in tools: + entry = TOOL_ANNOTATIONS.get(tool.name) + if entry is None: + continue + title, read_only, destructive, idempotent, open_world = entry + tool.title = title + tool.annotations = _tool_annotations( + title=title, + readOnlyHint=read_only, + destructiveHint=destructive, + idempotentHint=idempotent, + openWorldHint=open_world, + ) + return tools + -@server.list_tools() async def handle_list_tools() -> list[Tool]: """List available tools for AI agents""" - return [ - Tool( - name="connect_hardware", - description="Connect to remote hardware with automatic failover", - inputSchema={ - "type": "object", - "properties": { - "preferred_platform": { - "type": "string", - "description": "Preferred platform (windows, linux, docker)", - "enum": ["windows", "linux", "docker", "any"], - } - }, - }, - ), - Tool( - name="execute_command", - description="Execute command on remote hardware with full system access", - inputSchema={ - "type": "object", - "properties": { - "command": {"type": "string", "description": "Command to execute (full sudo access available)"}, - "timeout": {"type": "integer", "description": "Command timeout in seconds", "default": 30}, - "use_sudo": { - "type": "boolean", - "description": "Whether to use sudo for elevated privileges", - "default": False, - }, - "working_directory": { - "type": "string", - "description": "Working directory for command execution", - "default": "~", - }, - "environment": { - "type": "object", - "description": "Environment variables for command", - "default": {}, - }, - "bypass_security": { - "type": "boolean", - "description": "Bypass security validation for advanced AI agent operations", - "default": False, + return annotate( + [ + _tool( + name="connect_hardware", + description="Connect to remote hardware with automatic failover", + inputSchema={ + "type": "object", + "properties": { + "preferred_platform": { + "type": "string", + "description": "Preferred platform (windows, linux, docker)", + "enum": ["windows", "linux", "docker", "any"], + } }, }, - "required": ["command"], - }, - ), - Tool( - name="create_terminal_session", - description="Create a persistent interactive terminal session", - inputSchema={ - "type": "object", - "properties": { - "session_name": {"type": "string", "description": "Optional name for the terminal session"} - }, - }, - ), - Tool( - name="execute_in_terminal", - description="Execute command in existing terminal session (maintains state)", - inputSchema={ - "type": "object", - "properties": { - "session_id": {"type": "string", "description": "Terminal session ID"}, - "command": {"type": "string", "description": "Command to execute in session"}, - "timeout": {"type": "integer", "description": "Command timeout in seconds", "default": 30}, - "use_sudo": { - "type": "boolean", - "description": "Whether to use sudo for elevated privileges", - "default": False, + ), + _tool( + name="execute_command", + description="Execute command on remote hardware with full system access", + inputSchema={ + "type": "object", + "properties": { + "command": {"type": "string", "description": "Command to execute (full sudo access available)"}, + "timeout": {"type": "integer", "description": "Command timeout in seconds", "default": 30}, + "use_sudo": { + "type": "boolean", + "description": "Whether to use sudo for elevated privileges", + "default": False, + }, + "working_directory": { + "type": "string", + "description": "Working directory for command execution", + "default": "~", + }, + "environment": { + "type": "object", + "description": "Environment variables for command", + "default": {}, + }, + "bypass_security": { + "type": "boolean", + "description": "Bypass security validation for advanced AI agent operations", + "default": False, + }, }, - "bypass_security": { - "type": "boolean", - "description": "Bypass security validation for advanced AI agent operations", - "default": False, + "required": ["command"], + }, + ), + _tool( + name="create_terminal_session", + description="Create a persistent interactive terminal session", + inputSchema={ + "type": "object", + "properties": { + "session_name": {"type": "string", "description": "Optional name for the terminal session"} }, }, - "required": ["session_id", "command"], - }, - ), - Tool( - name="list_terminal_sessions", - description="List all active terminal sessions", - inputSchema={"type": "object", "properties": {}}, - ), - Tool( - name="close_terminal_session", - description="Close a terminal session", - inputSchema={ - "type": "object", - "properties": {"session_id": {"type": "string", "description": "Terminal session ID to close"}}, - "required": ["session_id"], - }, - ), - Tool( - name="get_hardware_info", - description="Get detailed hardware information from remote system", - inputSchema={ - "type": "object", - "properties": { - "refresh": { - "type": "boolean", - "description": "Force refresh hardware information", - "default": False, - } + ), + _tool( + name="execute_in_terminal", + description="Execute command in existing terminal session (maintains state)", + inputSchema={ + "type": "object", + "properties": { + "session_id": {"type": "string", "description": "Terminal session ID"}, + "command": {"type": "string", "description": "Command to execute in session"}, + "timeout": {"type": "integer", "description": "Command timeout in seconds", "default": 30}, + "use_sudo": { + "type": "boolean", + "description": "Whether to use sudo for elevated privileges", + "default": False, + }, + "bypass_security": { + "type": "boolean", + "description": "Bypass security validation for advanced AI agent operations", + "default": False, + }, + }, + "required": ["session_id", "command"], }, - }, - ), - Tool( - name="get_server_activity", - description="Read bounded, sanitized Rigout lifecycle status and recent activity", - inputSchema={ - "type": "object", - "properties": { - "lines": { - "type": "integer", - "description": "Number of recent activity lines to return", - "default": 50, - "minimum": 1, - "maximum": 200, - } + ), + _tool( + name="list_terminal_sessions", + description="List all active terminal sessions", + inputSchema={"type": "object", "properties": {}}, + ), + _tool( + name="close_terminal_session", + description="Close a terminal session", + inputSchema={ + "type": "object", + "properties": {"session_id": {"type": "string", "description": "Terminal session ID to close"}}, + "required": ["session_id"], }, - }, - ), - Tool( - name="manage_tunnels", - description="Manage tunnel endpoints (add, remove, test, failover)", - inputSchema={ - "type": "object", - "properties": { - "action": { - "type": "string", - "description": "Action to perform", - "enum": ["add", "remove", "test", "list", "failover"], - }, - "hostname": {"type": "string", "description": "Hostname for add/remove actions"}, - "username": {"type": "string", "description": "Username for SSH connection"}, - "private_key_path": {"type": "string", "description": "Path to SSH private key"}, - "port": { - "type": "integer", - "description": "SSH port for the add action (default 22)", - "minimum": 1, - "maximum": 65535, - "default": 22, - }, - "platform": { - "type": "string", - "description": "Platform type", - "enum": ["windows", "linux", "docker", "macos"], + ), + _tool( + name="get_hardware_info", + description="Get detailed hardware information from remote system", + inputSchema={ + "type": "object", + "properties": { + "refresh": { + "type": "boolean", + "description": "Force refresh hardware information", + "default": False, + } }, }, - "required": ["action"], - }, - ), - Tool( - name="install_software", - description="Install software packages on remote hardware", - inputSchema={ - "type": "object", - "properties": { - "packages": { - "type": "array", - "items": {"type": "string"}, - "description": "List of packages to install", - }, - "package_manager": { - "type": "string", - "description": "Package manager to use", - "enum": ["apt", "yum", "dnf", "pacman", "brew", "choco", "pip", "npm", "auto"], - "default": "auto", + ), + _tool( + name="get_server_activity", + description="Read bounded, sanitized Rigout lifecycle status and recent activity", + inputSchema={ + "type": "object", + "properties": { + "lines": { + "type": "integer", + "description": "Number of recent activity lines to return", + "default": 50, + "minimum": 1, + "maximum": 200, + } }, }, - "required": ["packages"], - }, - ), - Tool( - name="file_operations", - description="Perform file operations on remote hardware", - inputSchema={ - "type": "object", - "properties": { - "operation": { - "type": "string", - "description": "File operation to perform", - "enum": ["read", "write", "append", "delete", "copy", "move", "chmod", "chown"], - }, - "path": {"type": "string", "description": "File or directory path"}, - "content": {"type": "string", "description": "Content for write/append operations"}, - "destination": {"type": "string", "description": "Destination path for copy/move operations"}, - "permissions": {"type": "string", "description": "Permissions for chmod operation (e.g., '755')"}, - "owner": {"type": "string", "description": "Owner for chown operation (e.g., 'user:group')"}, - "recursive": { - "type": "boolean", - "description": "Required to delete a directory and everything inside it", - "default": False, + ), + _tool( + name="manage_tunnels", + description="Manage tunnel endpoints (add, remove, test, failover)", + inputSchema={ + "type": "object", + "properties": { + "action": { + "type": "string", + "description": "Action to perform", + "enum": ["add", "remove", "test", "list", "failover"], + }, + "hostname": {"type": "string", "description": "Hostname for add/remove actions"}, + "username": {"type": "string", "description": "Username for SSH connection"}, + "private_key_path": {"type": "string", "description": "Path to SSH private key"}, + "port": { + "type": "integer", + "description": "SSH port for the add action (default 22)", + "minimum": 1, + "maximum": 65535, + "default": 22, + }, + "platform": { + "type": "string", + "description": "Platform type", + "enum": ["windows", "linux", "docker", "macos"], + }, }, + "required": ["action"], }, - "required": ["operation", "path"], - }, - ), - Tool( - name="system_monitoring", - description="Monitor system resources and performance", - inputSchema={ - "type": "object", - "properties": { - "metrics": { - "type": "array", - "items": { + ), + _tool( + name="install_software", + description="Install software packages on remote hardware", + inputSchema={ + "type": "object", + "properties": { + "packages": { + "type": "array", + "items": {"type": "string"}, + "description": "List of packages to install", + }, + "package_manager": { "type": "string", - "enum": ["cpu", "memory", "disk", "network", "gpu", "processes", "all"], + "description": "Package manager to use", + "enum": ["apt", "yum", "dnf", "pacman", "brew", "choco", "pip", "npm", "auto"], + "default": "auto", }, - "description": "Metrics to monitor", - "default": ["all"], }, - "duration": {"type": "integer", "description": "Monitoring duration in seconds", "default": 10}, + "required": ["packages"], }, - }, - ), - Tool( - name="docker_operations", - description="Manage Docker containers and images for AI agent workflows", - inputSchema={ - "type": "object", - "properties": { - "operation": { - "type": "string", - "description": "Docker operation to perform", - "enum": ["list", "run", "exec", "stop", "remove", "build", "pull", "logs", "inspect"], + ), + _tool( + name="file_operations", + description="Perform file operations on remote hardware", + inputSchema={ + "type": "object", + "properties": { + "operation": { + "type": "string", + "description": "File operation to perform", + "enum": ["read", "write", "append", "delete", "copy", "move", "chmod", "chown"], + }, + "path": {"type": "string", "description": "File or directory path"}, + "content": {"type": "string", "description": "Content for write/append operations"}, + "destination": {"type": "string", "description": "Destination path for copy/move operations"}, + "permissions": { + "type": "string", + "description": "Permissions for chmod operation (e.g., '755')", + }, + "owner": {"type": "string", "description": "Owner for chown operation (e.g., 'user:group')"}, + "recursive": { + "type": "boolean", + "description": "Required to delete a directory and everything inside it", + "default": False, + }, }, - "container_name": {"type": "string", "description": "Container name or ID"}, - "image": {"type": "string", "description": "Docker image name"}, - "command": {"type": "string", "description": "Command to run in container"}, - "options": {"type": "object", "description": "Additional Docker options", "default": {}}, + "required": ["operation", "path"], }, - "required": ["operation"], - }, - ), - Tool( - name="bulk_file_transfer", - description="Transfer multiple files or directories for AI agent workflows", - inputSchema={ - "type": "object", - "properties": { - "operation": { - "type": "string", - "description": "Transfer operation", - "enum": ["upload", "download", "sync"], + ), + _tool( + name="system_monitoring", + description="Monitor system resources and performance", + inputSchema={ + "type": "object", + "properties": { + "metrics": { + "type": "array", + "items": { + "type": "string", + "enum": ["cpu", "memory", "disk", "network", "gpu", "processes", "all"], + }, + "description": "Metrics to monitor", + "default": ["all"], + }, + "duration": {"type": "integer", "description": "Monitoring duration in seconds", "default": 10}, }, - "source": {"type": "string", "description": "Source path or content"}, - "destination": {"type": "string", "description": "Destination path"}, - "files": {"type": "array", "items": {"type": "string"}, "description": "List of files to transfer"}, - "compress": {"type": "boolean", "description": "Compress files during transfer", "default": True}, }, - "required": ["operation", "source", "destination"], - }, - ), - Tool( - name="environment_setup", - description="Set up development environments for AI agent projects", - inputSchema={ - "type": "object", - "properties": { - "environment_type": { - "type": "string", - "description": "Type of environment to set up", - "enum": ["python", "node", "docker", "conda", "custom"], - }, - "requirements": { - "type": "array", - "items": {"type": "string"}, - "description": "List of requirements or dependencies", + ), + _tool( + name="docker_operations", + description="Manage Docker containers and images for AI agent workflows", + inputSchema={ + "type": "object", + "properties": { + "operation": { + "type": "string", + "description": "Docker operation to perform", + "enum": ["list", "run", "exec", "stop", "remove", "build", "pull", "logs", "inspect"], + }, + "container_name": {"type": "string", "description": "Container name or ID"}, + "image": {"type": "string", "description": "Docker image name"}, + "command": {"type": "string", "description": "Command to run in container"}, + "options": {"type": "object", "description": "Additional Docker options", "default": {}}, }, - "workspace_path": { - "type": "string", - "description": "Path to set up the workspace", - "default": "/tmp/ai_workspace", + "required": ["operation"], + }, + ), + _tool( + name="bulk_file_transfer", + description="Transfer multiple files or directories for AI agent workflows", + inputSchema={ + "type": "object", + "properties": { + "operation": { + "type": "string", + "description": "Transfer operation", + "enum": ["upload", "download", "sync"], + }, + "source": {"type": "string", "description": "Source path or content"}, + "destination": {"type": "string", "description": "Destination path"}, + "files": { + "type": "array", + "items": {"type": "string"}, + "description": "List of files to transfer", + }, + "compress": { + "type": "boolean", + "description": "Compress files during transfer", + "default": True, + }, }, - "configuration": { - "type": "object", - "description": "Additional configuration options", - "default": {}, + "required": ["operation", "source", "destination"], + }, + ), + _tool( + name="environment_setup", + description="Set up development environments for AI agent projects", + inputSchema={ + "type": "object", + "properties": { + "environment_type": { + "type": "string", + "description": "Type of environment to set up", + "enum": ["python", "node", "docker", "conda", "custom"], + }, + "requirements": { + "type": "array", + "items": {"type": "string"}, + "description": "List of requirements or dependencies", + }, + "workspace_path": { + "type": "string", + "description": "Path to set up the workspace", + "default": "/tmp/ai_workspace", + }, + "configuration": { + "type": "object", + "description": "Additional configuration options", + "default": {}, + }, }, + "required": ["environment_type"], }, - "required": ["environment_type"], - }, - ), - ] + ), + ] + ) async def _handle_call_tool_result(name: str, arguments: dict) -> CallToolResult: @@ -382,12 +494,12 @@ async def _dispatch_tool(name: str, arguments: dict) -> CallToolResult: elif name == "environment_setup": return await handle_environment_setup(arguments) else: - return CallToolResult( + return build_result( content=[TextContent(type="text", text=f"Unknown tool: {name}")], isError=True, ) except Exception as e: - return CallToolResult( + return build_result( content=[TextContent(type="text", text=f"Error executing tool '{name}': {str(e)}")], isError=True, ) @@ -415,15 +527,51 @@ def _error_message(name: str, content: Sequence[ContentBlock]) -> str: return "\n".join(parts) or f"Tool '{name}' failed" -@server.call_tool() async def handle_call_tool(name: str, arguments: dict) -> list[TextContent]: """Handle tool calls from MCP clients.""" result = await _handle_call_tool_result(name, arguments) - if result.isError: + if result_is_error(result): raise RuntimeError(_error_message(name, result.content)) return result.content # type: ignore +# mcp 1.x registers these with decorators; 2.x removed them for explicit registration. +# That is the entire incompatibility between the two majors - every tool definition above +# constructs unchanged, because 2.x renamed Tool's fields but kept the camelCase +# spellings as aliases, and stdio, streamable_http_manager, models and types all survive. +# +# Supporting both is deliberate rather than transitional. The fork is four lines wide and +# lets the dependency bound span both majors, so nobody is pushed onto a major the week it +# appears and nobody is stranded on the old one either. Which of the two is in use is +# decided by what is installed, not by a setting, so there is nothing to configure wrong. +def register_tool_handlers() -> None: + """Register the tool handlers against whichever mcp major is installed.""" + if hasattr(server, "list_tools"): + server.list_tools()(handle_list_tools) # type: ignore[attr-defined] + server.call_tool()(handle_call_tool) # type: ignore[attr-defined] + return + + from mcp.types import CallToolRequestParams, ListToolsResult, PaginatedRequestParams + + async def list_tools_request(_context: Any, _params: Any) -> ListToolsResult: + return ListToolsResult(tools=await handle_list_tools()) + + async def call_tool_request(_context: Any, params: Any) -> CallToolResult: + # Returned rather than raised. 1.x needs an error re-raised as RuntimeError for the + # SDK to rebuild it; 2.x takes the result as it is, so isError survives directly. + return await _handle_call_tool_result(params.name, params.arguments or {}) + + # Guarded by the hasattr above: this branch only runs on the major that has these, + # and the type checker sees whichever mcp is installed, so one of the two branches is + # always unknown to it. Ignoring here rather than loosening the annotation keeps the + # rest of the file checked. + server.add_request_handler("tools/list", PaginatedRequestParams, list_tools_request) # type: ignore[attr-defined] + server.add_request_handler("tools/call", CallToolRequestParams, call_tool_request) # type: ignore[attr-defined] + + +register_tool_handlers() + + async def handle_call_tool_result(name: str, arguments: dict) -> CallToolResult: """Compatibility helper for tests that need the full CallToolResult object.""" return await _handle_call_tool_result(name, arguments) diff --git a/src/rigout/tools/_results.py b/src/rigout/tools/_results.py index bdc8c8a..7960351 100644 --- a/src/rigout/tools/_results.py +++ b/src/rigout/tools/_results.py @@ -69,9 +69,22 @@ def transport_safe_result(result: CallToolResult) -> CallToolResult: return result +def build_result(**kwargs: Any) -> CallToolResult: + """Build a CallToolResult on either mcp major. + + Callers spell the flag `isError`. 2.x renamed the field to `is_error` and kept + `isError` as a construction alias, so this changes nothing that runs - it exists + because the type checker reads the field list and not the aliases. The read side of + the same rename is `result_is_error` just below. + """ + if "isError" in kwargs and "isError" not in CallToolResult.model_fields: + kwargs["is_error"] = kwargs.pop("isError") + return CallToolResult(**kwargs) + + def error_result(message: str) -> CallToolResult: """Return a tool result that MCP clients can reliably identify as failed.""" - return CallToolResult(content=[TextContent(type="text", text=message)], isError=True) + return build_result(content=[TextContent(type="text", text=message)], isError=True) def failure_detail(result: Mapping[str, Any], fallback: str = "Operation failed") -> str: @@ -90,3 +103,16 @@ def failure_detail(result: Mapping[str, Any], fallback: str = "Operation failed" if exit_code is not None: return f"Command exited with status {exit_code}" return fallback + + +def result_is_error(result: CallToolResult) -> bool: + """Whether a tool result is an error, on either mcp major. + + 2.x renamed the field to `is_error` while keeping `isError` as a construction alias, + so building a result works on both but reading `.isError` off one raises on 2.x. The + read is what has to be spelled carefully; the writes elsewhere do not. + """ + flag = getattr(result, "isError", None) + if flag is None: + flag = getattr(result, "is_error", None) + return bool(flag) diff --git a/tests/integration/test_mcp_server.py b/tests/integration/test_mcp_server.py index bf574af..3f43920 100644 --- a/tests/integration/test_mcp_server.py +++ b/tests/integration/test_mcp_server.py @@ -10,6 +10,31 @@ from rigout.security_validator import SecurityValidator from rigout.server import handle_call_tool, handle_call_tool_result, handle_list_tools, server from rigout.ssh_manager import TunnelEndpoint, get_tunnel_manager +from rigout.tools._results import result_is_error + + +async def call_via_sdk_handler(name: str, arguments: dict) -> object: + """Invoke the handler the SDK itself will invoke, on either mcp major. + + The registry is the one place the two majors differ, and it differs three ways: + 1.x keys `request_handlers` by request type, hands the handler the whole request, + and wraps the result in a `ServerResult`; 2.x exposes `get_request_handler` keyed + by method name, hands over `(context, params)`, and returns the result unwrapped. + + Same `hasattr` fork `register_tool_handlers()` makes in `server.py`, for the same + reason: what is installed decides, and there is nothing to configure. + """ + params = CallToolRequestParams(name=name, arguments=arguments) + + handlers = getattr(server, "request_handlers", None) + if handlers is not None: # mcp 1.x + response = await handlers[CallToolRequest](CallToolRequest(params=params)) + return response.root + + entry = server.get_request_handler("tools/call") # mcp 2.x + assert entry is not None, "no tools/call handler is registered" + handler = getattr(entry, "handler", entry) + return await handler(None, params) @pytest.mark.integration @@ -100,19 +125,16 @@ async def test_mock_tool_calls(self): # Test execute_command should fail gracefully when no endpoints are active with patch("rigout.ssh_manager.TunnelManager.auto_failover", return_value=None): result = await handle_call_tool_result("execute_command", {"command": "ls"}) - assert result.isError is True + assert result_is_error(result) is True assert "No available hardware endpoints" in result.content[0].text @pytest.mark.asyncio async def test_registered_handler_preserves_mcp_error_flag(self): """The SDK-facing handler must emit isError for unknown tools.""" - handler = server.request_handlers[CallToolRequest] - response = await handler( - CallToolRequest(params=CallToolRequestParams(name="definitely_unknown_tool", arguments={})) - ) + result = await call_via_sdk_handler("definitely_unknown_tool", {}) - assert response.root.isError is True - assert "Unknown tool" in response.root.content[0].text + assert result_is_error(result) is True + assert "Unknown tool" in result.content[0].text @pytest.mark.asyncio async def test_live_endpoint_when_configured(self): diff --git a/tests/unit/test_docs_truth.py b/tests/unit/test_docs_truth.py index e710ab4..61d8558 100644 --- a/tests/unit/test_docs_truth.py +++ b/tests/unit/test_docs_truth.py @@ -51,6 +51,21 @@ pytest.skip("documentation truth tests need the source checkout", allow_module_level=True) +def tool_input_schema(tool): + """A tool's input schema under either mcp major's spelling. + + 2.x renamed `inputSchema` to `input_schema` and kept the camelCase spelling as + a construction alias only, so reading `.inputSchema` raises there. Same shape + as `rigout.tools._results.result_is_error`, kept local because the server has + no reason to read a schema off its own tools. + """ + sentinel = object() + schema = getattr(tool, "inputSchema", sentinel) + if schema is sentinel: + schema = getattr(tool, "input_schema", None) + return schema + + def read(path: Path) -> str: """Read a repository text file with universal newlines, so CRLF checkouts match.""" return path.read_text(encoding="utf-8") @@ -264,7 +279,7 @@ def walk(node: object) -> None: walk(item) for tool in advertised_tools(): - walk(tool.inputSchema) + walk(tool_input_schema(tool)) return frozenset(words) @@ -290,7 +305,7 @@ def walk(node: object) -> None: walk(item) for tool in advertised_tools(): - walk(tool.inputSchema) + walk(tool_input_schema(tool)) return frozenset(names) @@ -1678,7 +1693,7 @@ def test_documented_setup_token_lifetime_matches_the_code(): def test_documented_activity_line_bounds_match_the_tool_schema(): """ "1-200 recent activity lines" is the advertised schema, not a wish.""" schema = next( - (tool.inputSchema for tool in advertised_tools() if tool.name == "get_server_activity"), + (tool_input_schema(tool) for tool in advertised_tools() if tool.name == "get_server_activity"), None, ) if not schema: diff --git a/tests/unit/test_server_activity.py b/tests/unit/test_server_activity.py index a152c26..f216cf1 100644 --- a/tests/unit/test_server_activity.py +++ b/tests/unit/test_server_activity.py @@ -13,6 +13,7 @@ write_json_secure, write_pid, ) +from rigout.tools._results import result_is_error from rigout.tools.activity import MAX_ACTIVITY_LINES, handle_get_server_activity @@ -42,7 +43,7 @@ async def test_server_activity_returns_bounded_sanitized_json(tmp_path, monkeypa result = await handle_get_server_activity({"lines": 3}) - assert result.isError is False + assert result_is_error(result) is False payload = json.loads(result.content[0].text) assert set(payload) == {"status", "running", "pid", "state_dir", "activity_log", "lines"} assert payload["status"] == "running" @@ -83,5 +84,5 @@ async def test_server_activity_does_not_leak_the_operator_home_directory(tmp_pat async def test_server_activity_rejects_unbounded_or_invalid_line_counts(line_count): result = await handle_get_server_activity({"lines": line_count}) - assert result.isError is True + assert result_is_error(result) is True assert "lines argument" in result.content[0].text diff --git a/tests/unit/test_server_metadata.py b/tests/unit/test_server_metadata.py index 7d1ee06..4092499 100644 --- a/tests/unit/test_server_metadata.py +++ b/tests/unit/test_server_metadata.py @@ -1,4 +1,5 @@ import ast +import asyncio import contextlib import os import subprocess @@ -15,14 +16,14 @@ @pytest.mark.unit def test_server_advertises_package_version(): - assert __version__ == "0.3.0" + assert __version__ == "0.3.1" assert server.version == __version__ @pytest.mark.unit def test_source_checkout_version_wins_over_stale_distribution_metadata(): with patch.object(_version, "distribution_version", return_value="0.1.0"): - assert _version.resolve_version() == "0.3.0" + assert _version.resolve_version() == "0.3.1" @pytest.mark.unit @@ -105,3 +106,87 @@ def test_import_does_not_create_a_cwd_log_file(tmp_path): ) assert not (tmp_path / "mcp-hardware-server.log").exists() + + +def hints(tool): + """Annotation hints by their wire names, on either mcp major. + + 2.x renamed the attributes to snake_case while keeping the camelCase spellings as + construction aliases, so reading `.readOnlyHint` off the model works on 1.x and + raises on 2.x. Dumping by alias gives the names the protocol actually uses, which + are the same on both. + """ + return tool.annotations.model_dump(by_alias=True) + + +@pytest.mark.unit +class TestToolAnnotations: + """Every tool must say what it does to the machine before a client runs it. + + Rigout spans the whole range - a tool that reads a CPU count and a tool that runs + arbitrary commands as root - and advertised them identically until now, leaving each + client to guess which was which. + """ + + @staticmethod + def _tools(): + return asyncio.run(rigout_server.handle_list_tools()) + + def test_every_advertised_tool_is_classified(self): + """A tool added without an entry ships unclassified, which is the failure this + test exists to prevent; the table is easy to forget and invisible when missed.""" + missing = [t.name for t in self._tools() if t.annotations is None] + + assert not missing, f"tools with no annotations: {missing}" + + def test_every_tool_has_a_human_readable_title(self): + assert all(t.title for t in self._tools()) + + def test_the_table_has_no_entries_for_tools_that_do_not_exist(self): + """A rename that updates one place and not the other leaves a dead entry and an + unclassified tool, and both are silent.""" + advertised = {t.name for t in self._tools()} + + assert set(rigout_server.TOOL_ANNOTATIONS) == advertised + + @pytest.mark.parametrize( + "name", + [ + "execute_command", + "execute_in_terminal", + "install_software", + "file_operations", + "docker_operations", + "bulk_file_transfer", + "environment_setup", + "manage_tunnels", + ], + ) + def test_tools_that_change_the_machine_are_marked_destructive(self, name): + tool = next(t for t in self._tools() if t.name == name) + + assert hints(tool)["readOnlyHint"] is False + assert hints(tool)["destructiveHint"] is True + + @pytest.mark.parametrize( + "name", ["get_hardware_info", "get_server_activity", "system_monitoring", "list_terminal_sessions"] + ) + def test_tools_that_only_look_are_marked_read_only(self, name): + tool = next(t for t in self._tools() if t.name == name) + + assert hints(tool)["readOnlyHint"] is True + assert hints(tool)["destructiveHint"] is False + + def test_anything_running_a_callers_command_is_not_idempotent(self): + """What such a tool does is decided by the caller and cannot be known here, so + claiming a repeat call changes nothing would be a guess stated as a fact.""" + for name in ("execute_command", "execute_in_terminal"): + tool = next(t for t in self._tools() if t.name == name) + assert hints(tool)["idempotentHint"] is False + + def test_read_only_tools_are_never_also_destructive(self): + """The two contradict each other, and a client reading either alone would be + told something untrue.""" + for tool in self._tools(): + if hints(tool)["readOnlyHint"]: + assert hints(tool)["destructiveHint"] is False, tool.name diff --git a/tests/unit/test_tool_handlers.py b/tests/unit/test_tool_handlers.py index 3727c77..b984c2c 100644 --- a/tests/unit/test_tool_handlers.py +++ b/tests/unit/test_tool_handlers.py @@ -29,6 +29,7 @@ handle_manage_tunnels, handle_system_monitoring, ) +from rigout.tools._results import result_is_error @pytest.mark.unit @@ -80,7 +81,7 @@ async def test_handle_execute_command_success(self, mock_manager): result = await handle_execute_command(args) assert isinstance(result, CallToolResult) - assert result.isError is False + assert result_is_error(result) is False assert "file1.txt" in result.content[0].text assert "Command executed successfully" in result.content[0].text mock_manager.execute_command.assert_called_once() @@ -101,7 +102,7 @@ async def test_handle_execute_command_failure(self, mock_manager): result = await handle_execute_command(args) assert isinstance(result, CallToolResult) - assert result.isError is True + assert result_is_error(result) is True assert "Command failed" in result.content[0].text assert "No such file or directory" in result.content[0].text @@ -125,7 +126,7 @@ async def test_handle_execute_command_failure_keeps_the_output_it_produced(self, result = await handle_execute_command({"command": "make && ./run-tests"}) text = result.content[0].text - assert result.isError is True + assert result_is_error(result) is True assert "compiled 41 objects" in text assert "3 tests failed" in text assert text.index("3 tests failed") < text.index("compiled 41 objects") @@ -144,7 +145,7 @@ async def test_handle_execute_command_failure_falls_back_to_exit_status(self, mo result = await handle_execute_command({"command": "false"}) - assert result.isError is True + assert result_is_error(result) is True assert "Command exited with status 1" in result.content[0].text async def test_handle_create_terminal_session(self, mock_manager): @@ -179,7 +180,7 @@ async def test_duplicate_session_name_says_so_instead_of_failing_generically(sel result = await handle_create_terminal_session({"session_name": "build"}) - assert result.isError is True + assert result_is_error(result) is True message = result.content[0].text assert "already exists" in message and "build" in message for way_out in ("execute_in_terminal", "close_terminal_session", "session_name"): @@ -224,7 +225,7 @@ async def test_handle_close_terminal_session(self, mock_manager): mock_manager.close_terminal_session.return_value = False result = await handle_close_terminal_session({"session_id": "sess-123"}) - assert result.isError is True + assert result_is_error(result) is True assert "Failed to close" in result.content[0].text async def test_handle_install_software(self, mock_manager): @@ -255,7 +256,7 @@ async def test_handle_install_software_pacman(self, mock_manager): args = {"packages": ["curl", "git"], "package_manager": "pacman"} result = await handle_install_software(args) - assert result.isError is False + assert result_is_error(result) is False assert "completed successfully" in result.content[0].text assert mock_manager.execute_command.call_args.args[1] == "sudo pacman -S --noconfirm curl git" @@ -288,7 +289,7 @@ async def test_handle_docker_failure_preserves_exit_status(self, mock_manager): result = await handle_docker_operations({"operation": "list"}) - assert result.isError is True + assert result_is_error(result) is True assert "Command exited with status 127" in result.content[0].text async def test_handle_docker_failure_keeps_the_output_it_produced(self, mock_manager): @@ -305,7 +306,7 @@ async def test_handle_docker_failure_keeps_the_output_it_produced(self, mock_man result = await handle_docker_operations({"operation": "logs", "container_name": "app"}) - assert result.isError is True + assert result_is_error(result) is True assert "OOMKilled" in result.content[0].text async def test_handle_environment_setup_failure_names_the_step_that_failed(self, mock_manager): @@ -328,7 +329,7 @@ async def test_handle_environment_setup_failure_names_the_step_that_failed(self, {"environment_type": "python", "workspace_path": "/tmp/ws", "requirements": ["numpy==999"]} ) - assert result.isError is True + assert result_is_error(result) is True assert "created venv" in result.content[0].text assert "no matching distribution" in result.content[0].text @@ -400,7 +401,7 @@ async def execute(_endpoint, command): result = await handle_system_monitoring({"metrics": ["cpu", "memory", "disk"]}) - assert result.isError is False + assert result_is_error(result) is False assert maximum_active > 1 assert mock_manager.execute_command.call_count == 3 @@ -419,7 +420,7 @@ async def execute(_endpoint, command): result = await handle_system_monitoring({"metrics": ["cpu", "memory"]}) - assert result.isError is True + assert result_is_error(result) is True assert "cpu available" in result.content[0].text assert "memory unavailable" in result.content[0].text @@ -552,7 +553,7 @@ async def test_local_terminal_output_is_sanitized(self, real_manager): result = await handle_execute_in_terminal({"session_id": "sess-local", "command": "env"}) - assert result.isError is False + assert result_is_error(result) is False assert "hunter2" not in result.content[0].text assert "sk-live-abcdef" not in result.content[0].text assert "password=***" in result.content[0].text @@ -564,7 +565,7 @@ async def test_ssh_terminal_output_is_sanitized(self, real_manager): result = await handle_execute_in_terminal({"session_id": "sess-ssh", "command": "env"}) - assert result.isError is False + assert result_is_error(result) is False assert "hunter2" not in result.content[0].text assert "abc123" not in result.content[0].text assert "password=***" in result.content[0].text @@ -577,7 +578,7 @@ async def test_destructive_command_in_terminal_is_rejected(self, real_manager): result = await handle_execute_in_terminal({"session_id": "sess-local", "command": "rm -rf /"}) - assert result.isError is True + assert result_is_error(result) is True assert "Security validation failed" in result.content[0].text assert session.executed == [] @@ -590,7 +591,7 @@ async def test_bypass_security_runs_destructive_command_in_terminal(self, real_m {"session_id": "sess-local", "command": "rm -rf /", "bypass_security": True} ) - assert result.isError is False + assert result_is_error(result) is False assert session.executed == ["rm -rf /"] async def test_sudo_in_terminal_requires_use_sudo(self, real_manager): @@ -602,7 +603,7 @@ async def test_sudo_in_terminal_requires_use_sudo(self, real_manager): {"session_id": "sess-local", "command": "sudo systemctl restart nginx"} ) - assert blocked.isError is True + assert result_is_error(blocked) is True assert "Sudo commands not allowed" in blocked.content[0].text assert session.executed == [] @@ -610,7 +611,7 @@ async def test_sudo_in_terminal_requires_use_sudo(self, real_manager): {"session_id": "sess-local", "command": "systemctl restart nginx", "use_sudo": True} ) - assert allowed.isError is False + assert result_is_error(allowed) is False assert session.executed == ["sudo systemctl restart nginx"] async def test_terminal_session_commands_are_rate_limited(self, real_manager): @@ -621,11 +622,11 @@ async def test_terminal_session_commands_are_rate_limited(self, real_manager): for _ in range(2): allowed = await handle_execute_in_terminal({"session_id": "sess-local", "command": "whoami"}) - assert allowed.isError is False + assert result_is_error(allowed) is False limited = await handle_execute_in_terminal({"session_id": "sess-local", "command": "whoami"}) - assert limited.isError is True + assert result_is_error(limited) is True assert "Rate limit exceeded" in limited.content[0].text assert session.executed == ["whoami", "whoami"] @@ -642,7 +643,7 @@ async def test_terminal_session_shares_the_endpoint_command_budget(self, real_ma limited = await handle_execute_in_terminal({"session_id": "sess-local", "command": "whoami"}) - assert limited.isError is True + assert result_is_error(limited) is True assert "Rate limit exceeded" in limited.content[0].text assert session.executed == [] @@ -771,7 +772,7 @@ async def test_local_delete_refuses_a_directory_without_recursive(self, local_ma result = await handle_file_operations({"operation": "delete", "path": str(target)}) - assert result.isError is True + assert result_is_error(result) is True assert "recursive=true" in result.content[0].text assert (target / "nested" / "keep.txt").exists() @@ -782,7 +783,7 @@ async def test_local_delete_removes_a_directory_when_asked(self, local_manager, result = await handle_file_operations({"operation": "delete", "path": str(target), "recursive": True}) - assert result.isError is False + assert result_is_error(result) is False assert not target.exists() async def test_remote_delete_is_recursive_only_when_asked(self, remote_manager): diff --git a/tests/unit/test_tool_platform.py b/tests/unit/test_tool_platform.py index a71582d..5a854d3 100644 --- a/tests/unit/test_tool_platform.py +++ b/tests/unit/test_tool_platform.py @@ -21,6 +21,7 @@ is_windows_platform, platform_family, ) +from rigout.tools._results import result_is_error from rigout.tools.command import handle_install_software from rigout.tools.docker import handle_docker_operations from rigout.tools.environment import handle_environment_setup @@ -132,7 +133,7 @@ async def test_macos_monitoring_never_runs_powershell(self): with patch_manager("monitoring", manager): result = await handle_system_monitoring({"metrics": ["all"]}) - assert result.isError is False + assert result_is_error(result) is False assert manager.commands, "no monitoring commands were built" assert "powershell" not in manager.joined.lower() assert "Get-CimInstance" not in manager.joined @@ -219,7 +220,7 @@ async def test_no_if_guard_swallows_the_setup_chain(self, tmp_path): ) command = manager.only_command - assert result.isError is False + assert result_is_error(result) is False assert "if not exist" not in command assert command.startswith(f'cd /d "{workspace}"') assert "python -m venv venv" in command @@ -232,7 +233,7 @@ async def test_default_workspace_is_a_usable_windows_path(self, tmp_path, monkey with patch_manager("environment", manager): result = await handle_environment_setup({"environment_type": "python"}) - assert result.isError is False + assert result_is_error(result) is False assert "/tmp/ai_workspace" not in manager.only_command assert str(tmp_path / "ai_workspace") in manager.only_command assert (tmp_path / "ai_workspace").is_dir(), "the workspace was never created" @@ -253,7 +254,7 @@ async def test_unmappable_posix_path_is_refused_with_guidance(self, tmp_path): {"environment_type": "python", "workspace_path": "/home/agent/work"} ) - assert result.isError is True + assert result_is_error(result) is True assert "POSIX absolute path" in text_of(result) assert manager.commands == [], "a doomed command was sent anyway" @@ -270,7 +271,7 @@ async def test_docker_dockerfile_is_written_not_shell_quoted(self, tmp_path): } ) - assert result.isError is False + assert result_is_error(result) is False assert "powershell" not in manager.only_command.lower() dockerfile = workspace / "Dockerfile" assert dockerfile.is_file() @@ -289,7 +290,7 @@ async def test_unquotable_requirement_cannot_break_out_of_the_command(self, tmp_ } ) - assert result.isError is True + assert result_is_error(result) is True assert "cannot be quoted for cmd.exe" in text_of(result) assert manager.commands == [] @@ -305,7 +306,7 @@ async def test_pip_specifiers_are_still_accepted(self, tmp_path): } ) - assert result.isError is False + assert result_is_error(result) is False assert 'venv\\Scripts\\python.exe -m pip install "numpy>=1.26,<2"' in manager.only_command async def test_windows_path_translation(self, tmp_path, monkeypatch): @@ -329,7 +330,7 @@ async def test_macos_local_setup_uses_posix_syntax(self, tmp_path): result = await handle_environment_setup({"environment_type": "python", "workspace_path": str(workspace)}) command = manager.only_command - assert result.isError is False + assert result_is_error(result) is False assert "if not exist" not in command assert "cd /d" not in command assert "python3 -m venv venv" in command @@ -446,7 +447,7 @@ async def test_remove_closes_pooled_clients_and_sessions(self): with patch_manager("tunnel", manager): result = await handle_manage_tunnels({"action": "remove", "hostname": "gone.example.com"}) - assert result.isError is False + assert result_is_error(result) is False assert client.closed is True, "a pooled SSH client stayed open and authenticated" assert session.closed is True, "a terminal session still reaches the removed host" assert "gone.example.com:22" not in manager._connection_pool @@ -470,7 +471,7 @@ async def test_missing_hostname_is_a_clear_error_not_a_keyerror(self): with patch_manager("tunnel", manager): result = await handle_manage_tunnels({"action": "add", "username": "agent"}) - assert result.isError is True + assert result_is_error(result) is True assert "hostname" in text_of(result) assert manager.added == [] @@ -527,7 +528,7 @@ async def test_an_unusable_port_is_refused_with_a_reason(self, tmp_path, bad_por } ) - assert result.isError is True + assert result_is_error(result) is True assert "port" in text_of(result).lower() assert manager.added == [], "a refused endpoint must not be registered" @@ -558,7 +559,7 @@ async def test_connection_is_actually_tested_and_reported(self, tmp_path): ) assert len(manager.tested) == 1, "add claimed to test the connection but never did" - assert result.isError is False + assert result_is_error(result) is False assert "connection test PASSED" in text_of(result) async def test_failed_connection_test_is_reported_as_failure(self, tmp_path): @@ -573,7 +574,7 @@ async def test_failed_connection_test_is_reported_as_failure(self, tmp_path): } ) - assert result.isError is True + assert result_is_error(result) is True assert "connection test FAILED" in text_of(result) async def test_missing_private_key_is_refused(self, tmp_path): @@ -588,7 +589,7 @@ async def test_missing_private_key_is_refused(self, tmp_path): } ) - assert result.isError is True + assert result_is_error(result) is True assert "Private key not found" in text_of(result) assert manager.added == [], "an endpoint with an unusable key was saved anyway" @@ -604,7 +605,7 @@ async def test_duplicate_hostname_is_refused(self, tmp_path): } ) - assert result.isError is True + assert result_is_error(result) is True assert "already configured" in text_of(result) assert manager.added == [] @@ -653,7 +654,7 @@ async def test_non_integer_duration_is_rejected(self): with patch_manager("monitoring", manager): result = await handle_system_monitoring({"metrics": ["cpu"], "duration": "60"}) - assert result.isError is True + assert result_is_error(result) is True assert "must be an integer" in text_of(result) assert manager.commands == [] diff --git a/tests/unit/test_transport_safety.py b/tests/unit/test_transport_safety.py index aaeb303..4348a4f 100644 --- a/tests/unit/test_transport_safety.py +++ b/tests/unit/test_transport_safety.py @@ -20,6 +20,7 @@ from rigout.tools._results import ( MAX_RESULT_CHARS, error_result, + result_is_error, transport_safe_result, transport_safe_text, ) @@ -85,7 +86,7 @@ def test_error_results_are_cleaned_too(): """An error carrying raw bytes breaks the stream exactly as a success does.""" cleaned = transport_safe_result(error_result("failed on \x00\x01 input")) - assert cleaned.isError is True + assert result_is_error(cleaned) is True assert "\x00" not in cleaned.content[0].text