diff --git a/docs/cli/tg-set-tool.md b/docs/cli/tg-set-tool.md index 74f8bbcd..883d4c8b 100644 --- a/docs/cli/tg-set-tool.md +++ b/docs/cli/tg-set-tool.md @@ -31,9 +31,9 @@ The command updates both the tool index and stores the complete tool configurati - Must be unique within the tool registry - `--name NAME` - - **Required.** Human-readable name for the tool - - Displayed in tool listings and user interfaces - - Should be descriptive and clear + - **Required.** Tool name used by agents to invoke this tool + - Must be a valid function identifier (use snake_case, no spaces or special characters) + - Examples: `get_weather`, `calculate_distance`, `search_documents` - `--type TYPE` - **Required.** Tool type defining its functionality @@ -63,7 +63,7 @@ The command updates both the tool index and stores the complete tool configurati Register a simple weather lookup tool: ```bash -tg-set-tool --id weather --name "Weather Lookup" \ +tg-set-tool --id weather_tool --name get_weather \ --type knowledge-query \ --description "Get current weather information" \ --argument location:string:"Location to query" \ @@ -74,7 +74,8 @@ tg-set-tool --id weather --name "Weather Lookup" \ Register a calculator tool with MCP type: ```bash -tg-set-tool --id calculator --name "Calculator" --type mcp-tool \ +tg-set-tool --id calc_tool --name calculate \ + --type mcp-tool \ --description "Perform mathematical calculations" \ --argument expression:string:"Mathematical expression to evaluate" ``` @@ -83,7 +84,7 @@ tg-set-tool --id calculator --name "Calculator" --type mcp-tool \ Register a text completion tool: ```bash -tg-set-tool --id text-generator --name "Text Generator" \ +tg-set-tool --id text_gen_tool --name generate_text \ --type text-completion \ --description "Generate text based on prompts" \ --argument prompt:string:"Text prompt for generation" \ @@ -95,7 +96,7 @@ tg-set-tool --id text-generator --name "Text Generator" \ Register a tool with custom API endpoint: ```bash tg-set-tool -u http://trustgraph.example.com:8088/ \ - --id custom-tool --name "Custom Tool" \ + --id custom_tool --name custom_search \ --type knowledge-query \ --description "Custom tool functionality" ``` @@ -104,7 +105,7 @@ tg-set-tool -u http://trustgraph.example.com:8088/ \ Register a simple tool with no arguments: ```bash -tg-set-tool --id status-check --name "Status Check" \ +tg-set-tool --id status_tool --name check_status \ --type knowledge-query \ --description "Check system status" ``` diff --git a/docs/tech-specs/MCP_TOOL_ARGUMENTS.md b/docs/tech-specs/MCP_TOOL_ARGUMENTS.md new file mode 100644 index 00000000..9b0c8560 --- /dev/null +++ b/docs/tech-specs/MCP_TOOL_ARGUMENTS.md @@ -0,0 +1,256 @@ +# MCP Tool Arguments Specification + +## Overview +**Feature Name**: MCP Tool Arguments Support +**Author**: Claude Code Assistant +**Date**: 2025-08-21 +**Status**: Finalised + +### Executive Summary + +Enable ReACT agents to invoke MCP (Model Context Protocol) tools with +properly defined arguments by adding argument specification support to +MCP tool configurations, similar to how prompt template tools +currently work. + +### Problem Statement + +Currently, MCP tools in the ReACT agent framework cannot specify their +expected arguments. The `McpToolImpl.get_arguments()` method returns +an empty list, forcing LLMs to guess the correct parameter structure +based only on tool names and descriptions. This leads to: +- Unreliable tool invocations due to parameter guessing +- Poor user experience when tools fail due to incorrect arguments +- No validation of tool parameters before execution +- Missing parameter documentation in agent prompts + +### Goals + +- [ ] Allow MCP tool configurations to specify expected arguments (name, type, description) +- [ ] Update agent manager to expose MCP tool arguments to LLMs via prompts +- [ ] Maintain backward compatibility with existing MCP tool configurations +- [ ] Support argument validation similar to prompt template tools + +### Non-Goals +- Dynamic argument discovery from MCP servers (future enhancement) +- Argument type validation beyond basic structure +- Complex argument schemas (nested objects, arrays) + +## Background and Context + +### Current State +MCP tools are configured in the ReACT agent system with minimal metadata: +```json +{ + "type": "mcp-tool", + "name": "get_bank_balance", + "description": "Get bank account balance", + "mcp-tool": "get_bank_balance" +} +``` + +The `McpToolImpl.get_arguments()` method returns `[]`, so LLMs receive no argument guidance in their prompts. + +### Limitations + +1. **No argument specification**: MCP tools cannot define expected + parameters + +2. **LLM parameter guessing**: Agents must infer parameters from tool + names/descriptions + +3. **Missing prompt information**: Agent prompts show no argument + details for MCP tools + +4. **No validation**: Invalid parameters are only caught at MCP tool + execution time + +### Related Components +- **trustgraph-flow/agent/react/service.py**: Tool configuration loading and AgentManager creation +- **trustgraph-flow/agent/react/tools.py**: McpToolImpl implementation +- **trustgraph-flow/agent/react/agent_manager.py**: Prompt generation with tool arguments +- **trustgraph-cli**: CLI tools for MCP tool management +- **Workbench**: External UI for agent tool configuration + +## Requirements + +### Functional Requirements + +1. **MCP Tool Configuration Arguments**: MCP tool configurations MUST support an optional `arguments` array with name, type, and description fields +2. **Argument Exposure**: `McpToolImpl.get_arguments()` MUST return configured arguments instead of empty list +3. **Prompt Integration**: Agent prompts MUST include MCP tool argument details when arguments are specified +4. **Backward Compatibility**: Existing MCP tool configurations without arguments MUST continue to work +5. **CLI Support**: Existing `tg-invoke-mcp-tool` CLI supports arguments (already implemented) + +### Non-Functional Requirements +1. **Backward Compatibility**: Zero breaking changes for existing MCP tool configurations +2. **Performance**: No significant performance impact on agent prompt generation +3. **Consistency**: Argument handling MUST match prompt template tool patterns + +### User Stories + +1. As an **agent developer**, I want to specify MCP tool arguments in configuration so that LLMs can invoke tools with correct parameters +2. As a **workbench user**, I want to configure MCP tool arguments in the UI so that agents use tools properly +3. As an **LLM in a ReACT agent**, I want to see tool argument specifications in prompts so that I can provide correct parameters + +## Design + +### High-Level Architecture +Extend MCP tool configuration to match the prompt template pattern by: +1. Adding optional `arguments` array to MCP tool configurations +2. Modifying `McpToolImpl` to accept and return configured arguments +3. Updating tool configuration loading to handle MCP tool arguments +4. Ensuring agent prompts include MCP tool argument information + +### Configuration Schema +```json +{ + "type": "mcp-tool", + "name": "get_bank_balance", + "description": "Get bank account balance", + "mcp-tool": "get_bank_balance", + "arguments": [ + { + "name": "account_id", + "type": "string", + "description": "Bank account identifier" + }, + { + "name": "date", + "type": "string", + "description": "Date for balance query (optional, format: YYYY-MM-DD)" + } + ] +} +``` + +### Data Flow +1. **Configuration Loading**: MCP tool config with arguments is loaded by `on_tools_config()` +2. **Tool Creation**: Arguments are parsed and passed to `McpToolImpl` via constructor +3. **Prompt Generation**: `agent_manager.py` calls `tool.arguments` to include in LLM prompts +4. **Tool Invocation**: LLM provides parameters which are passed to MCP service unchanged + +### API Changes +No external API changes - this is purely internal configuration and argument handling. + +### Component Details + +#### Component 1: service.py (Tool Configuration Loading) +- **Purpose**: Parse MCP tool configurations and create tool instances +- **Changes Required**: Add argument parsing for MCP tools (similar to prompt tools) +- **New Functionality**: Extract `arguments` array from MCP tool config and create `Argument` objects + +#### Component 2: tools.py (McpToolImpl) +- **Purpose**: MCP tool implementation wrapper +- **Changes Required**: Accept arguments in constructor and return them from `get_arguments()` +- **New Functionality**: Store and expose configured arguments instead of returning empty list + +#### Component 3: Workbench (External Repository) +- **Purpose**: UI for configuring agent tools +- **Changes Required**: Add argument specification UI for MCP tools +- **New Functionality**: Allow users to add/edit/remove arguments for MCP tools + +#### Component 4: CLI Tools +- **Purpose**: Command-line tool management +- **Changes Required**: Support argument specification in MCP tool creation/update commands +- **New Functionality**: Accept arguments parameter in tool configuration commands + +## Implementation Plan + +### Phase 1: Core Agent Framework Changes +- [ ] Update `McpToolImpl` constructor to accept `arguments` parameter +- [ ] Change `McpToolImpl.get_arguments()` to return stored arguments +- [ ] Modify `service.py` MCP tool configuration parsing to handle arguments +- [ ] Add unit tests for MCP tool argument handling +- [ ] Verify agent prompts include MCP tool arguments + +### Phase 2: External Tool Support +- [ ] Update CLI tools to support MCP tool argument specification +- [ ] Document argument configuration format for users +- [ ] Update Workbench UI to support MCP tool argument configuration +- [ ] Add examples and documentation + +### Code Changes Summary +| File | Change Type | Description | +|------|------------|-------------| +| `tools.py` | Modified | Update McpToolImpl to accept and store arguments | +| `service.py` | Modified | Parse arguments from MCP tool config (line 108-113) | +| `test_react_processor.py` | Modified | Add tests for MCP tool arguments | +| CLI tools | Modified | Support argument specification in commands | +| Workbench | Modified | Add UI for MCP tool argument configuration | + +## Testing Strategy + +### Unit Tests +- **MCP Tool Argument Parsing**: Test `service.py` correctly parses arguments from MCP tool configurations +- **McpToolImpl Arguments**: Test `get_arguments()` returns configured arguments instead of empty list +- **Backward Compatibility**: Test MCP tools without arguments continue to work (return empty list) +- **Agent Prompt Generation**: Test agent prompts include MCP tool argument details + +### Integration Tests +- **End-to-End Tool Invocation**: Test agent with MCP tool arguments can successfully invoke tools +- **Configuration Loading**: Test complete config load cycle with MCP tool arguments +- **Cross-Component**: Test arguments flow correctly from config → tool creation → prompt generation + +### Manual Testing +- **Agent Behavior**: Manually verify LLM receives and uses argument information in ReACT cycles +- **CLI Integration**: Test tg-invoke-mcp-tool works with new argument-configured MCP tools +- **Workbench Integration**: Test UI supports MCP tool argument configuration + +## Migration and Rollout + +### Migration Strategy +No migration required - this is purely additive functionality: +- Existing MCP tool configurations without `arguments` continue to work unchanged +- `McpToolImpl.get_arguments()` returns empty list for legacy tools +- New configurations can optionally include `arguments` array + +### Rollout Plan +1. **Phase 1**: Deploy core agent framework changes to development/staging +2. **Phase 2**: Deploy CLI tool updates and documentation +3. **Phase 3**: Deploy Workbench UI updates for argument configuration +4. **Phase 4**: Production rollout with monitoring + +### Rollback Plan +- Core changes are backward compatible - no rollback needed for functionality +- If issues arise, disable argument parsing by reverting MCP tool config loading logic +- Workbench and CLI changes are independent and can be rolled back separately + +## Security Considerations +- **No new attack surface**: Arguments are parsed from existing configuration sources with no new inputs +- **Parameter validation**: Arguments are passed through to MCP tools unchanged - validation remains at MCP tool level +- **Configuration integrity**: Argument specifications are part of tool configuration - same security model applies + +## Performance Impact +- **Minimal overhead**: Argument parsing happens only during configuration loading, not per-request +- **Prompt size increase**: Agent prompts will include MCP tool argument details, slightly increasing token usage +- **Memory usage**: Negligible increase for storing argument specifications in tool objects + +## Documentation + +### User Documentation +- [ ] Update MCP tool configuration guide with argument examples +- [ ] Add argument specification to CLI tool help text +- [ ] Create examples of common MCP tool argument patterns + +### Developer Documentation +- [ ] Update McpToolImpl class documentation +- [ ] Add inline comments for argument parsing logic +- [ ] Document argument flow in system architecture + +## Open Questions +1. **Argument validation**: Should we validate argument types/formats beyond basic structure checking? +2. **Dynamic discovery**: Future enhancement to query MCP servers for tool schemas automatically? + +## Alternatives Considered +1. **Dynamic MCP schema discovery**: Query MCP servers for tool argument schemas at runtime - rejected due to complexity and reliability concerns +2. **Separate argument registry**: Store MCP tool arguments in separate configuration section - rejected for consistency with prompt template approach +3. **Type validation**: Full JSON schema validation for arguments - deferred as future enhancement to keep initial implementation simple + +## References +- [MCP Protocol Specification](https://github.com/modelcontextprotocol/spec) +- [Prompt Template Tool Implementation](./trustgraph-flow/trustgraph/agent/react/service.py#L114-129) +- [Current MCP Tool Implementation](./trustgraph-flow/trustgraph/agent/react/tools.py#L58-86) + +## Appendix +[Any additional information, diagrams, or examples] diff --git a/tests/unit/test_agent/test_react_processor.py b/tests/unit/test_agent/test_react_processor.py index 32b2625b..028f416f 100644 --- a/tests/unit/test_agent/test_react_processor.py +++ b/tests/unit/test_agent/test_react_processor.py @@ -758,6 +758,186 @@ Answer: The capital of France is Paris.""" assert clean_action in tools, \ f"Cleaned action '{clean_action}' from '{quoted_action}' should be in tools" + def test_mcp_tool_arguments_support(self): + """Test that MCP tools can be configured with arguments and expose them correctly + + This test verifies the MCP tool arguments feature where: + 1. MCP tool configurations can specify arguments + 2. Configuration parsing extracts arguments correctly + 3. Arguments are structured properly for tool use + """ + # Define a simple Argument class for testing (mimics the real one) + class TestArgument: + def __init__(self, name, type, description): + self.name = name + self.type = type + self.description = description + + # Define a mock McpToolImpl that mimics the new functionality + class MockMcpToolImpl: + def __init__(self, context, mcp_tool_id, arguments=None): + self.context = context + self.mcp_tool_id = mcp_tool_id + self.arguments = arguments or [] + + def get_arguments(self): + return self.arguments + + # Test 1: MCP tool with arguments + test_arguments = [ + TestArgument( + name="account_id", + type="string", + description="Bank account identifier" + ), + TestArgument( + name="date", + type="string", + description="Date for balance query (optional, format: YYYY-MM-DD)" + ) + ] + + context_mock = lambda service_name: None + mcp_tool_with_args = MockMcpToolImpl( + context=context_mock, + mcp_tool_id="get_bank_balance", + arguments=test_arguments + ) + + returned_args = mcp_tool_with_args.get_arguments() + + # Verify arguments are stored and returned correctly + assert len(returned_args) == 2 + assert returned_args[0].name == "account_id" + assert returned_args[0].type == "string" + assert returned_args[0].description == "Bank account identifier" + assert returned_args[1].name == "date" + assert returned_args[1].type == "string" + assert "optional" in returned_args[1].description.lower() + + # Test 2: MCP tool without arguments (backward compatibility) + mcp_tool_no_args = MockMcpToolImpl( + context=context_mock, + mcp_tool_id="simple_tool" + ) + + returned_args_empty = mcp_tool_no_args.get_arguments() + assert len(returned_args_empty) == 0 + assert returned_args_empty == [] + + # Test 3: MCP tool with empty arguments list + mcp_tool_empty_args = MockMcpToolImpl( + context=context_mock, + mcp_tool_id="another_tool", + arguments=[] + ) + + returned_args_explicit_empty = mcp_tool_empty_args.get_arguments() + assert len(returned_args_explicit_empty) == 0 + assert returned_args_explicit_empty == [] + + # Test 4: Configuration parsing simulation + def simulate_config_parsing(config_data): + """Simulate how service.py parses MCP tool configuration""" + config_args = config_data.get("arguments", []) + arguments = [ + TestArgument( + name=arg.get("name"), + type=arg.get("type"), + description=arg.get("description") + ) + for arg in config_args + ] + return arguments + + # Test configuration with arguments + config_with_args = { + "type": "mcp-tool", + "name": "get_bank_balance", + "description": "Get bank account balance", + "mcp-tool": "get_bank_balance", + "arguments": [ + { + "name": "account_id", + "type": "string", + "description": "Bank account identifier" + }, + { + "name": "date", + "type": "string", + "description": "Date for balance query (optional)" + } + ] + } + + parsed_args = simulate_config_parsing(config_with_args) + assert len(parsed_args) == 2 + assert parsed_args[0].name == "account_id" + assert parsed_args[1].name == "date" + + # Test configuration without arguments + config_without_args = { + "type": "mcp-tool", + "name": "simple_tool", + "description": "Simple MCP tool", + "mcp-tool": "simple_tool" + } + + parsed_args_empty = simulate_config_parsing(config_without_args) + assert len(parsed_args_empty) == 0 + + # Test 5: Argument structure validation + def validate_argument_structure(arg): + """Validate that an argument has required fields""" + required_fields = ['name', 'type', 'description'] + return all(hasattr(arg, field) and getattr(arg, field) for field in required_fields) + + # Validate all parsed arguments have proper structure + for arg in parsed_args: + assert validate_argument_structure(arg), f"Argument {arg.name} missing required fields" + + # Test 6: Prompt template integration simulation + def simulate_prompt_template_rendering(tools): + """Simulate how agent prompts include tool arguments""" + tool_descriptions = [] + + for tool in tools: + tool_desc = f"- **{tool.name}**: {tool.description}" + + # Add argument details if present + for arg in tool.arguments: + tool_desc += f"\n - Required: `\"{arg.name}\"` ({arg.type}): {arg.description}" + + tool_descriptions.append(tool_desc) + + return "\n".join(tool_descriptions) + + # Create mock tools with our MCP tool + class MockTool: + def __init__(self, name, description, arguments): + self.name = name + self.description = description + self.arguments = arguments + + mock_tools = [ + MockTool("search", "Search the web", []), # Tool without arguments + MockTool("get_bank_balance", "Get bank account balance", parsed_args) # MCP tool with arguments + ] + + prompt_section = simulate_prompt_template_rendering(mock_tools) + + # Verify the prompt includes MCP tool arguments + assert "get_bank_balance" in prompt_section + assert "account_id" in prompt_section + assert "Bank account identifier" in prompt_section + assert "date" in prompt_section + assert "(string)" in prompt_section + assert "Required:" in prompt_section + + # Verify tools without arguments still work + assert "search" in prompt_section + assert "Search the web" in prompt_section + def test_error_handling_in_react_cycle(self): """Test error handling during ReAct execution""" # Arrange diff --git a/trustgraph-cli/trustgraph/cli/set_tool.py b/trustgraph-cli/trustgraph/cli/set_tool.py index ca86c9be..e39dfad7 100644 --- a/trustgraph-cli/trustgraph/cli/set_tool.py +++ b/trustgraph-cli/trustgraph/cli/set_tool.py @@ -9,6 +9,10 @@ This script allows you to define agent tools with various types including: Tools are stored in the 'tool' configuration group and can include argument specifications for parameterized execution. + +IMPORTANT: The tool 'name' is used by agents to invoke the tool and must +be a valid function identifier (use snake_case, no spaces or special chars). +The 'description' provides human-readable information about the tool. """ from typing import List @@ -114,14 +118,15 @@ def main(): number - Numeric parameter Examples: - %(prog)s --id weather --name "Weather lookup" \\ + %(prog)s --id weather_tool --name get_weather \\ --type knowledge-query \\ - --description "Get weather information" \\ + --description "Get weather information for a location" \\ --argument location:string:"Location to query" \\ --argument units:string:"Temperature units (C/F)" - %(prog)s --id calculator --name "Calculator" --type mcp-tool \\ - --description "Perform calculations" \\ + %(prog)s --id calc_tool --name calculate --type mcp-tool \\ + --description "Perform mathematical calculations" \\ + --mcp-tool calculator \\ --argument expression:string:"Mathematical expression" ''').strip(), formatter_class=argparse.RawDescriptionHelpFormatter @@ -140,7 +145,7 @@ def main(): parser.add_argument( '--name', - help=f'Human-readable tool name', + help=f'Tool name used by agents to invoke this tool (use snake_case, e.g., get_weather)', ) parser.add_argument( diff --git a/trustgraph-flow/trustgraph/agent/react/service.py b/trustgraph-flow/trustgraph/agent/react/service.py index c148519e..74b89a1e 100755 --- a/trustgraph-flow/trustgraph/agent/react/service.py +++ b/trustgraph-flow/trustgraph/agent/react/service.py @@ -106,11 +106,21 @@ class Processor(AgentService): impl = TextCompletionImpl arguments = TextCompletionImpl.get_arguments() elif impl_id == "mcp-tool": + # For MCP tools, arguments come from config (similar to prompt tools) + config_args = data.get("arguments", []) + arguments = [ + Argument( + name=arg.get("name"), + type=arg.get("type"), + description=arg.get("description") + ) + for arg in config_args + ] impl = functools.partial( McpToolImpl, - mcp_tool_id=data.get("mcp-tool") + mcp_tool_id=data.get("mcp-tool"), + arguments=arguments ) - arguments = McpToolImpl.get_arguments() elif impl_id == "prompt": # For prompt tools, arguments come from config config_args = data.get("arguments", []) diff --git a/trustgraph-flow/trustgraph/agent/react/tools.py b/trustgraph-flow/trustgraph/agent/react/tools.py index e1a2af85..d2a15bba 100644 --- a/trustgraph-flow/trustgraph/agent/react/tools.py +++ b/trustgraph-flow/trustgraph/agent/react/tools.py @@ -57,15 +57,14 @@ class TextCompletionImpl: # the mcp-tool service. class McpToolImpl: - def __init__(self, context, mcp_tool_id): + def __init__(self, context, mcp_tool_id, arguments=None): self.context = context self.mcp_tool_id = mcp_tool_id + self.arguments = arguments or [] - @staticmethod - def get_arguments(): - # MCP tools define their own arguments dynamically - # For now, we return empty list and let the MCP service handle validation - return [] + def get_arguments(self): + # Return configured arguments if available, otherwise empty list for backward compatibility + return self.arguments async def invoke(self, **arguments):