diff --git a/docs/schema_reference.md b/docs/schema_reference.md index 68d5edb..9b8345f 100644 --- a/docs/schema_reference.md +++ b/docs/schema_reference.md @@ -617,6 +617,91 @@ execution: url: "https://api.example.com/resources/{{props.id}}" ``` +### Input Schema and Default Values + +The `inputSchema` field uses JSON Schema to define the expected properties for a tool. When executing a tool, the MCI adapter processes properties as follows: + +1. **Required Properties**: Must be provided, or execution will fail with a validation error +2. **Optional Properties with Defaults**: If not provided, the default value is used +3. **Optional Properties without Defaults**: If not provided, they are skipped (not included in template context) + +This behavior prevents template substitution errors for optional properties that aren't needed for a particular execution. + +#### Example: Properties with Defaults + +```json +{ + "name": "search_files", + "description": "Search for text in files", + "inputSchema": { + "type": "object", + "properties": { + "pattern": { + "type": "string", + "description": "Search pattern" + }, + "directory": { + "type": "string", + "description": "Directory to search in" + }, + "include_images": { + "type": "boolean", + "description": "Include image files in search", + "default": false + }, + "case_sensitive": { + "type": "boolean", + "description": "Use case-sensitive search", + "default": true + }, + "max_results": { + "type": "number", + "description": "Maximum number of results", + "default": 100 + }, + "file_extensions": { + "type": "string", + "description": "Optional comma-separated list of file extensions" + } + }, + "required": ["pattern", "directory"] + }, + "execution": { + "type": "text", + "text": "Searching '{{props.pattern}}' in {{props.directory}} (images: {{props.include_images}}, max: {{props.max_results}})" + } +} +``` + +**Execution with minimal properties:** +```python +# Only required properties provided +client.execute("search_files", properties={ + "pattern": "TODO", + "directory": "/home/user/projects" +}) +# Result: include_images=false, case_sensitive=true, max_results=100 (defaults used) +# file_extensions is skipped (not in template, no default) +``` + +**Execution with overridden defaults:** +```python +# Some defaults overridden +client.execute("search_files", properties={ + "pattern": "FIXME", + "directory": "/tmp", + "include_images": true, + "max_results": 50 +}) +# Result: include_images=true, max_results=50 (overridden), case_sensitive=true (default) +``` + +**Property Resolution Rules:** +- Properties provided at execution time always take precedence over defaults +- Default values can be any valid JSON type: boolean, number, string, array, object, null +- Optional properties without defaults are not included in the template context if not provided +- This prevents `{{props.optional_prop}}` from causing errors when `optional_prop` is not provided + --- ## Execution Types diff --git a/src/mcipy/mcp_integration.py b/src/mcipy/mcp_integration.py index aa33c79..7b7e87e 100644 --- a/src/mcipy/mcp_integration.py +++ b/src/mcipy/mcp_integration.py @@ -5,7 +5,8 @@ fetching their tool definitions, and building MCI-compatible toolset schemas. """ -import asyncio, concurrent.futures +import asyncio +import concurrent.futures from datetime import UTC, datetime, timedelta from typing import Any @@ -102,6 +103,7 @@ def fetch_and_build_toolset( - If a loop IS running (e.g., inside an async CLI), offload the async work to a separate thread that owns its own loop, and block until it finishes. """ + async def _coro(): return await MCPIntegration.fetch_and_build_toolset_async( server_name, server_config, schema_version, env_context, template_engine diff --git a/src/mcipy/tool_manager.py b/src/mcipy/tool_manager.py index 7e2abfd..5b8bb01 100644 --- a/src/mcipy/tool_manager.py +++ b/src/mcipy/tool_manager.py @@ -227,12 +227,17 @@ def execute( # This handles three cases: None (no schema), {} (empty schema), and {...} (schema with properties) if tool.inputSchema is not None and tool.inputSchema: self._validate_input_properties(tool, properties) + # Resolve properties with defaults applied and optional properties skipped + resolved_properties = self._resolve_properties_with_defaults(tool, properties) + else: + # No schema, use properties as-is + resolved_properties = properties # Build context for execution context: dict[str, Any] = { - "props": properties, + "props": resolved_properties, "env": env_vars, - "input": properties, # Alias for backward compatibility + "input": resolved_properties, # Alias for backward compatibility } # Build path validation context @@ -299,3 +304,58 @@ def _validate_input_properties(self, tool: Tool, properties: dict[str, Any]) -> f"Tool '{tool.name}' requires properties: {', '.join(required)}. " f"Missing: {', '.join(missing_props)}" ) + + def _resolve_properties_with_defaults( + self, tool: Tool, properties: dict[str, Any] + ) -> dict[str, Any]: + """ + Resolve properties with default values and skip optional properties. + + For each property in the input schema: + - If provided in properties: use the provided value + - Else if has default value in schema: use the default + - Else if required: already validated, should not happen + - Else (optional without default): skip, don't include in resolved properties + + This prevents template substitution errors for optional properties that + are not provided and have no default value. + + Args: + tool: Tool object with inputSchema + properties: Properties provided by the caller + + Returns: + Resolved properties dictionary with defaults applied and optional properties skipped + """ + input_schema = tool.inputSchema + if not input_schema: + return properties + + # Get schema properties definition + schema_properties = input_schema.get("properties", {}) + if not schema_properties: + # No properties defined in schema, return as-is + return properties + + # Get required properties list + required = set(input_schema.get("required", [])) + + # Build resolved properties + resolved: dict[str, Any] = {} + + # Process each property in the schema + for prop_name, prop_schema in schema_properties.items(): + if prop_name in properties: + # Property was provided, use it + resolved[prop_name] = properties[prop_name] + elif "default" in prop_schema: + # Property not provided but has default, use default + resolved[prop_name] = prop_schema["default"] + elif prop_name in required: + # Required property not provided - this should have been caught by validation + # but we'll include it anyway to maintain consistency + # (validation should have raised an error before we get here) + pass + # else: optional property without default - skip it + + return resolved diff --git a/tests/test_default_values_integration.py b/tests/test_default_values_integration.py new file mode 100644 index 0000000..cee0ab2 --- /dev/null +++ b/tests/test_default_values_integration.py @@ -0,0 +1,274 @@ +""" +Integration tests for default value support and optional property handling. + +These tests demonstrate the real-world usage of the default value feature +where tools can define default values for optional properties, and those +defaults are used when properties are not provided. +""" + +import pytest + +from mcipy import Annotations, MCISchema, TextExecutionConfig, Tool, ToolManager + + +class TestDefaultValuesIntegration: + """Integration tests for default values feature.""" + + @pytest.fixture + def schema_with_search_tool(self): + """Create a schema with a search tool similar to the issue description.""" + tools = [ + Tool( + name="search_files", + annotations=Annotations(title="Search Files", readOnlyHint=True), + description="Search for text in files with optional parameters", + inputSchema={ + "type": "object", + "properties": { + "pattern": { + "type": "string", + "description": "Search pattern", + }, + "directory": { + "type": "string", + "description": "Directory to search in", + }, + "include_images": { + "type": "boolean", + "description": "Include image files in search", + "default": False, + }, + "case_sensitive": { + "type": "boolean", + "description": "Use case-sensitive search", + "default": True, + }, + "max_results": { + "type": "number", + "description": "Maximum number of results", + "default": 100, + }, + "file_extensions": { + "type": "string", + "description": "Optional comma-separated list of file extensions", + }, + }, + "required": ["pattern", "directory"], + }, + execution=TextExecutionConfig( + text="Searching for '{{props.pattern}}' in {{props.directory}}\n" + "Include images: {{props.include_images}}\n" + "Case sensitive: {{props.case_sensitive}}\n" + "Max results: {{props.max_results}}" + ), + ) + ] + return MCISchema(schemaVersion="1.0", tools=tools) + + def test_required_properties_only_uses_defaults(self, schema_with_search_tool): + """Test that providing only required properties uses defaults for optional ones.""" + manager = ToolManager(schema_with_search_tool) + + # Execute with only required properties + result = manager.execute( + "search_files", + properties={ + "pattern": "TODO", + "directory": "/home/user/projects", + }, + ) + + # Should succeed + assert result.result.isError is False + content = result.result.content[0].text + + # Verify defaults are used + assert "Searching for 'TODO' in /home/user/projects" in content + assert "Include images: False" in content + assert "Case sensitive: True" in content + assert "Max results: 100" in content + + def test_override_some_defaults(self, schema_with_search_tool): + """Test that some defaults can be overridden while others are used.""" + manager = ToolManager(schema_with_search_tool) + + # Execute with some optional properties overridden + result = manager.execute( + "search_files", + properties={ + "pattern": "FIXME", + "directory": "/tmp", + "include_images": True, + "max_results": 50, + }, + ) + + assert result.result.isError is False + content = result.result.content[0].text + + # Verify provided values and defaults + assert "Searching for 'FIXME' in /tmp" in content + assert "Include images: True" in content # Overridden + assert "Case sensitive: True" in content # Default + assert "Max results: 50" in content # Overridden + + def test_optional_property_without_default_does_not_cause_error( + self, schema_with_search_tool + ): + """Test that optional properties without defaults don't cause template errors.""" + manager = ToolManager(schema_with_search_tool) + + # Execute without providing file_extensions (optional, no default) + # This should not cause a template error even though file_extensions is not in the template + result = manager.execute( + "search_files", + properties={ + "pattern": "ERROR", + "directory": "/var/log", + }, + ) + + # Should succeed without attempting to resolve file_extensions + assert result.result.isError is False + + def test_all_properties_provided(self, schema_with_search_tool): + """Test that all properties can be provided, overriding all defaults.""" + manager = ToolManager(schema_with_search_tool) + + # Execute with all properties + result = manager.execute( + "search_files", + properties={ + "pattern": "WARNING", + "directory": "/var/log", + "include_images": False, + "case_sensitive": False, + "max_results": 10, + "file_extensions": ".log,.txt", + }, + ) + + assert result.result.isError is False + content = result.result.content[0].text + + # Verify all provided values + assert "Searching for 'WARNING' in /var/log" in content + assert "Include images: False" in content + assert "Case sensitive: False" in content + assert "Max results: 10" in content + + def test_boolean_default_false(self, schema_with_search_tool): + """Test that boolean default value of False works correctly.""" + manager = ToolManager(schema_with_search_tool) + + result = manager.execute( + "search_files", + properties={ + "pattern": "test", + "directory": "/tmp", + }, + ) + + assert result.result.isError is False + content = result.result.content[0].text + # include_images has default=False + assert "Include images: False" in content + + def test_number_default(self, schema_with_search_tool): + """Test that number default value works correctly.""" + manager = ToolManager(schema_with_search_tool) + + result = manager.execute( + "search_files", + properties={ + "pattern": "test", + "directory": "/tmp", + }, + ) + + assert result.result.isError is False + content = result.result.content[0].text + # max_results has default=100 + assert "Max results: 100" in content + + def test_string_default_when_used(self): + """Test string default values.""" + tool = Tool( + name="format_text", + annotations=Annotations(title="Format Text"), + inputSchema={ + "type": "object", + "properties": { + "text": {"type": "string"}, + "format": {"type": "string", "default": "plain"}, + }, + "required": ["text"], + }, + execution=TextExecutionConfig( + text="Text: {{props.text}}, Format: {{props.format}}" + ), + ) + schema = MCISchema(schemaVersion="1.0", tools=[tool]) + manager = ToolManager(schema) + + result = manager.execute("format_text", properties={"text": "Hello"}) + + assert result.result.isError is False + content = result.result.content[0].text + assert "Text: Hello, Format: plain" in content + + +class TestBackwardCompatibility: + """Tests to ensure backward compatibility with existing behavior.""" + + def test_no_input_schema_still_works(self): + """Test that tools without input schema still work.""" + tool = Tool( + name="simple_tool", + annotations=Annotations(title="Simple Tool"), + inputSchema=None, + execution=TextExecutionConfig(text="Simple output"), + ) + schema = MCISchema(schemaVersion="1.0", tools=[tool]) + manager = ToolManager(schema) + + result = manager.execute("simple_tool", properties={"any_prop": "value"}) + assert result.result.isError is False + + def test_empty_input_schema_still_works(self): + """Test that tools with empty input schema still work.""" + tool = Tool( + name="empty_schema_tool", + annotations=Annotations(title="Empty Schema Tool"), + inputSchema={}, + execution=TextExecutionConfig(text="Output: {{props.custom}}"), + ) + schema = MCISchema(schemaVersion="1.0", tools=[tool]) + manager = ToolManager(schema) + + result = manager.execute("empty_schema_tool", properties={"custom": "value"}) + assert result.result.isError is False + + def test_required_validation_still_works(self): + """Test that required property validation still works.""" + from mcipy import ToolManagerError + + tool = Tool( + name="required_tool", + annotations=Annotations(title="Required Tool"), + inputSchema={ + "type": "object", + "properties": {"required_prop": {"type": "string"}}, + "required": ["required_prop"], + }, + execution=TextExecutionConfig(text="Output"), + ) + schema = MCISchema(schemaVersion="1.0", tools=[tool]) + manager = ToolManager(schema) + + # Should raise error when required property is missing + with pytest.raises(ToolManagerError) as exc_info: + manager.execute("required_tool", properties={}) + + assert "requires properties" in str(exc_info.value) + assert "required_prop" in str(exc_info.value) diff --git a/tests/unit/test_tool_manager.py b/tests/unit/test_tool_manager.py index d8dd897..376d84b 100644 --- a/tests/unit/test_tool_manager.py +++ b/tests/unit/test_tool_manager.py @@ -824,3 +824,254 @@ def test_without_tags_filter_with_disabled_tools(self): assert len(filtered) == 1 assert "enabled_no_api" in tool_names assert "disabled_no_api" not in tool_names + + +class TestDefaultValuesAndOptionalProperties: + """Tests for default value support and optional property handling.""" + + @pytest.fixture + def schema_with_defaults(self): + """Create a schema with tools that have default values and optional properties.""" + from mcipy import Annotations + + tools = [ + Tool( + name="tool_with_defaults", + annotations=Annotations(title="Tool With Defaults"), + description="Tool with default values", + inputSchema={ + "type": "object", + "properties": { + "required_prop": { + "type": "string", + "description": "Required property", + }, + "optional_with_default": { + "type": "boolean", + "description": "Optional with default", + "default": False, + }, + "optional_no_default": { + "type": "string", + "description": "Optional without default", + }, + "another_default": { + "type": "string", + "description": "Another property with default", + "default": "default_value", + }, + }, + "required": ["required_prop"], + }, + execution=TextExecutionConfig( + text="Required: {{props.required_prop}}, " + "OptionalWithDefault: {{props.optional_with_default}}, " + "AnotherDefault: {{props.another_default}}" + ), + ), + Tool( + name="tool_all_optional_with_defaults", + annotations=Annotations(title="All Optional With Defaults"), + description="Tool where all properties have defaults", + inputSchema={ + "type": "object", + "properties": { + "prop1": {"type": "string", "default": "default1"}, + "prop2": {"type": "number", "default": 42}, + "prop3": {"type": "boolean", "default": True}, + }, + "required": [], + }, + execution=TextExecutionConfig( + text="Prop1: {{props.prop1}}, Prop2: {{props.prop2}}, Prop3: {{props.prop3}}" + ), + ), + ] + return MCISchema(schemaVersion="1.0", tools=tools) + + def test_default_value_used_when_property_not_provided(self, schema_with_defaults): + """Test that default values are used when properties are not provided.""" + manager = ToolManager(schema_with_defaults) + + # Execute with only required property + result = manager.execute( + "tool_with_defaults", properties={"required_prop": "test_value"} + ) + + assert result.result.isError is False + content_text = result.result.content[0].text + # Should use default values for optional_with_default and another_default + assert "Required: test_value" in content_text + assert "OptionalWithDefault: False" in content_text + assert "AnotherDefault: default_value" in content_text + + def test_provided_value_overrides_default(self, schema_with_defaults): + """Test that provided values override default values.""" + manager = ToolManager(schema_with_defaults) + + # Execute with all properties provided + result = manager.execute( + "tool_with_defaults", + properties={ + "required_prop": "test_value", + "optional_with_default": True, + "another_default": "custom_value", + }, + ) + + assert result.result.isError is False + content_text = result.result.content[0].text + # Should use provided values, not defaults + assert "Required: test_value" in content_text + assert "OptionalWithDefault: True" in content_text + assert "AnotherDefault: custom_value" in content_text + + def test_optional_property_without_default_is_skipped(self, schema_with_defaults): + """Test that optional properties without defaults don't cause template errors.""" + manager = ToolManager(schema_with_defaults) + + # Execute with only required property (optional_no_default not provided) + # This should not raise an error, even though the template doesn't reference it + result = manager.execute( + "tool_with_defaults", properties={"required_prop": "test_value"} + ) + + # The execution should succeed + assert result.result.isError is False + + def test_all_defaults_used_when_no_properties_provided(self, schema_with_defaults): + """Test that all default values are used when no properties are provided.""" + manager = ToolManager(schema_with_defaults) + + # Execute with no properties at all + result = manager.execute("tool_all_optional_with_defaults", properties={}) + + assert result.result.isError is False + content_text = result.result.content[0].text + # Should use all default values + assert "Prop1: default1" in content_text + assert "Prop2: 42" in content_text + assert "Prop3: True" in content_text + + def test_partial_override_of_defaults(self, schema_with_defaults): + """Test that some properties can override defaults while others use defaults.""" + manager = ToolManager(schema_with_defaults) + + # Execute with only one property overridden + result = manager.execute( + "tool_all_optional_with_defaults", properties={"prop1": "custom1"} + ) + + assert result.result.isError is False + content_text = result.result.content[0].text + # Should use custom value for prop1, defaults for others + assert "Prop1: custom1" in content_text + assert "Prop2: 42" in content_text + assert "Prop3: True" in content_text + + def test_default_values_with_different_types(self, schema_with_defaults): + """Test that default values work correctly for different property types.""" + manager = ToolManager(schema_with_defaults) + + result = manager.execute("tool_all_optional_with_defaults", properties={}) + + assert result.result.isError is False + # Verify the resolved properties have the correct types + # This is an integration test - we just verify execution succeeds + + def test_resolve_properties_with_defaults_method(self, schema_with_defaults): + """Test the _resolve_properties_with_defaults method directly.""" + manager = ToolManager(schema_with_defaults) + tool = manager.get_tool("tool_with_defaults") + + # Test with only required property + resolved = manager._resolve_properties_with_defaults( + tool, {"required_prop": "test"} + ) + + assert resolved["required_prop"] == "test" + assert resolved["optional_with_default"] is False + assert resolved["another_default"] == "default_value" + # optional_no_default should not be in resolved (skipped) + assert "optional_no_default" not in resolved + + def test_resolve_properties_all_provided(self, schema_with_defaults): + """Test property resolution when all properties are provided.""" + manager = ToolManager(schema_with_defaults) + tool = manager.get_tool("tool_with_defaults") + + # Test with all properties provided + resolved = manager._resolve_properties_with_defaults( + tool, + { + "required_prop": "test", + "optional_with_default": True, + "optional_no_default": "provided", + "another_default": "custom", + }, + ) + + assert resolved["required_prop"] == "test" + assert resolved["optional_with_default"] is True + assert resolved["optional_no_default"] == "provided" + assert resolved["another_default"] == "custom" + + def test_resolve_properties_no_schema(self): + """Test property resolution when tool has no input schema.""" + from mcipy import Annotations + + tool = Tool( + name="no_schema_tool", + annotations=Annotations(title="No Schema"), + inputSchema=None, + execution=TextExecutionConfig(text="Test"), + ) + schema = MCISchema(schemaVersion="1.0", tools=[tool]) + manager = ToolManager(schema) + + resolved = manager._resolve_properties_with_defaults( + tool, {"custom_prop": "value"} + ) + + # Should return properties as-is when no schema + assert resolved == {"custom_prop": "value"} + + def test_resolve_properties_empty_schema(self): + """Test property resolution when tool has empty input schema.""" + from mcipy import Annotations + + tool = Tool( + name="empty_schema_tool", + annotations=Annotations(title="Empty Schema"), + inputSchema={}, + execution=TextExecutionConfig(text="Test"), + ) + schema = MCISchema(schemaVersion="1.0", tools=[tool]) + manager = ToolManager(schema) + + resolved = manager._resolve_properties_with_defaults( + tool, {"custom_prop": "value"} + ) + + # Should return properties as-is when schema is empty + assert resolved == {"custom_prop": "value"} + + def test_resolve_properties_schema_without_properties(self): + """Test property resolution when schema has no properties field.""" + from mcipy import Annotations + + tool = Tool( + name="no_props_tool", + annotations=Annotations(title="No Props"), + inputSchema={"type": "object"}, + execution=TextExecutionConfig(text="Test"), + ) + schema = MCISchema(schemaVersion="1.0", tools=[tool]) + manager = ToolManager(schema) + + resolved = manager._resolve_properties_with_defaults( + tool, {"custom_prop": "value"} + ) + + # Should return properties as-is when no properties in schema + assert resolved == {"custom_prop": "value"} diff --git a/testsManual/test_default_values.py b/testsManual/test_default_values.py new file mode 100644 index 0000000..be61417 --- /dev/null +++ b/testsManual/test_default_values.py @@ -0,0 +1,128 @@ +""" +Manual test for default values feature. + +Run this directly to see the feature in action. +""" + +import json +import tempfile +from pathlib import Path + +from mcipy import MCIClient + +# Create a schema with default values +schema_dict = { + "schemaVersion": "1.0", + "tools": [ + { + "name": "search_files", + "annotations": { + "title": "Search Files", + "readOnlyHint": True, + }, + "description": "Search for text in files with optional parameters", + "inputSchema": { + "type": "object", + "properties": { + "pattern": { + "type": "string", + "description": "Search pattern", + }, + "directory": { + "type": "string", + "description": "Directory to search in", + }, + "include_images": { + "type": "boolean", + "description": "Include image files in search", + "default": False, + }, + "case_sensitive": { + "type": "boolean", + "description": "Use case-sensitive search", + "default": True, + }, + "max_results": { + "type": "number", + "description": "Maximum number of results", + "default": 100, + }, + "file_extensions": { + "type": "string", + "description": "Optional comma-separated list of file extensions", + }, + }, + "required": ["pattern", "directory"], + }, + "execution": { + "type": "text", + "text": "Searching for '{{props.pattern}}' in {{props.directory}}\n" + "Include images: {{props.include_images}}\n" + "Case sensitive: {{props.case_sensitive}}\n" + "Max results: {{props.max_results}}", + }, + } + ], +} + +# Write to temp file +with tempfile.NamedTemporaryFile(mode="w", suffix=".mci.json", delete=False) as f: + json.dump(schema_dict, f) + temp_file = f.name + +try: + client = MCIClient(schema_file_path=temp_file) + + print("=" * 60) + print("Test 1: Execute with only required properties") + print("=" * 60) + result = client.execute( + "search_files", + properties={ + "pattern": "TODO", + "directory": "/home/user/projects", + }, + ) + print(result.result.content[0].text) + print() + + print("=" * 60) + print("Test 2: Execute with some defaults overridden") + print("=" * 60) + result = client.execute( + "search_files", + properties={ + "pattern": "FIXME", + "directory": "/tmp", + "include_images": True, + "max_results": 50, + }, + ) + print(result.result.content[0].text) + print() + + print("=" * 60) + print("Test 3: Execute with all defaults overridden") + print("=" * 60) + result = client.execute( + "search_files", + properties={ + "pattern": "ERROR", + "directory": "/var/log", + "include_images": False, + "case_sensitive": False, + "max_results": 10, + "file_extensions": ".log,.txt", + }, + ) + print(result.result.content[0].text) + print() + + print("=" * 60) + print("SUCCESS! All tests passed.") + print("Default values are working correctly!") + print("=" * 60) + +finally: + # Clean up temp file + Path(temp_file).unlink(missing_ok=True)