diff --git a/src/itential_mcp/runtime/constants.py b/src/itential_mcp/runtime/constants.py index 89695afe..7ee2540d 100644 --- a/src/itential_mcp/runtime/constants.py +++ b/src/itential_mcp/runtime/constants.py @@ -58,6 +58,7 @@ class CommandConfig: "metavar": "", "help": "Parameters to pass to the tool", }, + "--config": {"help": CONFIG_HELP_MESSAGE}, }, add_platform_group=True, ), diff --git a/src/itential_mcp/runtime/parser.py b/src/itential_mcp/runtime/parser.py index 8e350271..6598684a 100644 --- a/src/itential_mcp/runtime/parser.py +++ b/src/itential_mcp/runtime/parser.py @@ -146,8 +146,23 @@ def parse_args(args: Sequence) -> Tuple[Callable, Tuple[Any, ...], dict]: parser = _create_main_parser() _create_subparsers(parser) + # Pre-capture --config from raw args before argparse parsing. + # When --config appears before the subcommand (e.g., itential-mcp --config + # file.conf call ...), the main parser captures it but the subparser + # overwrites it with its default (None). This preserves the value. + pre_config = None + args_list = list(args) + if "--config" in args_list: + idx = args_list.index("--config") + if idx + 1 < len(args_list): + pre_config = args_list[idx + 1] + parsed_args = parser.parse_args(args=args) + # Restore --config if the subparser overwrote it with None + if parsed_args.config is None and pre_config is not None: + parsed_args.config = pre_config + _process_logging_config(parsed_args) if parsed_args.help or parsed_args.command is None: diff --git a/src/itential_mcp/runtime/runner.py b/src/itential_mcp/runtime/runner.py index 4260979a..4a141a64 100644 --- a/src/itential_mcp/runtime/runner.py +++ b/src/itential_mcp/runtime/runner.py @@ -77,5 +77,10 @@ async def run(tool: str, params: Mapping[str, Any] | None = None) -> None: # Execute operations result = await client.call_tool(tool, **kwargs) - data = json.loads(result.content[0].text) - print(f"\n{json.dumps(data, indent=4)}") + text = result.content[0].text + + try: + data = json.loads(text) + print(f"\n{json.dumps(data, indent=4)}") + except json.JSONDecodeError: + print(f"\n{text}") diff --git a/src/itential_mcp/tools/gateway_manager.py b/src/itential_mcp/tools/gateway_manager.py index 97d88031..702b808a 100644 --- a/src/itential_mcp/tools/gateway_manager.py +++ b/src/itential_mcp/tools/gateway_manager.py @@ -156,7 +156,7 @@ async def run_service( if isinstance(input_params, str): input_params = jsonutils.loads(input_params) - res = await client.gateway_manager.run_service(name, cluster, input_params) + res = await client.gateway_manager.run_service(name, cluster, input_params=input_params) if "error" in res: raise ValueError(res["error"]["data"]) diff --git a/tests/test_commands.py b/tests/test_commands.py index cec7ae18..105f702a 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -371,6 +371,81 @@ def __init__(self, tool, params): assert result[1] == ("my_tool", '{"test": true}') + def test_call_with_config_attribute(self): + """Test that call function works when args has config attribute""" + + class MockArgs: + def __init__(self, tool, params, config): + self.tool = tool + self.params = params + self.config = config + + args = MockArgs("my_tool", '{"test": true}', "/path/to/config.conf") + result = commands.call(args) + + # call command should still return the same tuple structure + assert result[0] == runner.run + assert result[1] == ("my_tool", '{"test": true}') + assert result[2] is None + + +class TestCallCommandConfigIntegration: + """Test cases for call command --config integration""" + + def test_call_subparser_accepts_config_flag(self): + """Test that the call subparser accepts --config flag""" + from unittest.mock import patch + from itential_mcp.runtime.parser import _create_main_parser, _create_subparsers + + parser = _create_main_parser() + with patch("itential_mcp.runtime.parser.add_platform_group"), \ + patch("itential_mcp.runtime.parser.add_server_group"): + _create_subparsers(parser) + + # Should parse successfully with --config on call subcommand + args = parser.parse_args(["call", "get_health", "--config", "/path/to/config.conf"]) + + assert args.command == "call" + assert args.tool == "get_health" + assert args.config == "/path/to/config.conf" + + def test_call_subparser_config_default_is_none(self): + """Test that --config defaults to None when not provided""" + from unittest.mock import patch + from itential_mcp.runtime.parser import _create_main_parser, _create_subparsers + + parser = _create_main_parser() + with patch("itential_mcp.runtime.parser.add_platform_group"), \ + patch("itential_mcp.runtime.parser.add_server_group"): + _create_subparsers(parser) + + args = parser.parse_args(["call", "get_health"]) + + assert args.command == "call" + assert args.tool == "get_health" + assert args.config is None + + def test_call_subparser_config_with_params(self): + """Test that --config works alongside --params on call subcommand""" + from unittest.mock import patch + from itential_mcp.runtime.parser import _create_main_parser, _create_subparsers + + parser = _create_main_parser() + with patch("itential_mcp.runtime.parser.add_platform_group"), \ + patch("itential_mcp.runtime.parser.add_server_group"): + _create_subparsers(parser) + + args = parser.parse_args([ + "call", "get_devices", + "--config", "/path/to/config.conf", + "--params", '{"filter": "active"}', + ]) + + assert args.command == "call" + assert args.tool == "get_devices" + assert args.config == "/path/to/config.conf" + assert args.params == '{"filter": "active"}' + class TestModuleStructure: """Test cases for overall module structure and imports""" diff --git a/tests/test_runner.py b/tests/test_runner.py index 52172217..b4079800 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -699,3 +699,54 @@ async def test_run_full_workflow( assert '"name": "John Doe"' in output assert '"status": "success"' in output assert '"role": "admin"' in output + + @pytest.mark.asyncio + @patch("sys.stdout", new_callable=StringIO) + @patch("itential_mcp.runtime.runner.Client") + @patch("itential_mcp.runtime.runner.Server") + @patch("itential_mcp.runtime.runner.config.get") + async def test_run_non_json_result_output( + self, mock_config_get, mock_server_class, mock_client_class, mock_stdout + ): + """Test that non-JSON results (e.g. TOON format) are printed directly""" + # Setup mocks + mock_config = MagicMock() + mock_config_get.return_value = mock_config + + # Setup server instance mock + mock_server_instance = MagicMock() + mock_server_instance.mcp = MagicMock() + mock_server_instance.__aenter__ = AsyncMock(return_value=mock_server_instance) + mock_server_instance.__aexit__ = AsyncMock(return_value=None) + mock_server_class.return_value = mock_server_instance + + mock_client = AsyncMock() + mock_client.ping.return_value = True + + # Mock tool list response + mock_tool = MagicMock() + mock_tool.name = "test_tool" + mock_tool.inputSchema = {"properties": {}, "required": []} + + mock_list_response = MagicMock() + mock_list_response.tools = [mock_tool] + mock_client.list_tools_mcp.return_value = mock_list_response + + # Mock TOON-formatted result (not valid JSON) + toon_result = "status: running\nhost: selab-ip6-dev\nservices[2]:\n redis: running\n mongo: running" + mock_result_content = MagicMock() + mock_result_content.text = toon_result + mock_result = MockCallToolResult(content=[mock_result_content]) + mock_client.call_tool.return_value = mock_result + + mock_client_class.return_value.__aenter__.return_value = mock_client + mock_client_class.return_value.__aexit__.return_value = None + + # Execute + await runner.run("test_tool") + + # Verify TOON output is printed directly + output = mock_stdout.getvalue() + assert "status: running" in output + assert "host: selab-ip6-dev" in output + assert "redis: running" in output diff --git a/tests/test_tools_gateway_manager.py b/tests/test_tools_gateway_manager.py index 283cbfba..42760af6 100644 --- a/tests/test_tools_gateway_manager.py +++ b/tests/test_tools_gateway_manager.py @@ -459,7 +459,7 @@ async def test_run_service_success_basic(self): # Verify client method was called with correct parameters self.mock_client.gateway_manager.run_service.assert_called_once_with( - "test-service", "test-cluster", {"param1": "value1"} + "test-service", "test-cluster", input_params={"param1": "value1"} ) # Verify result type and content @@ -489,7 +489,7 @@ async def test_run_service_success_no_params(self): # Verify client method was called with None for input_params self.mock_client.gateway_manager.run_service.assert_called_once_with( - "no-param-service", "test-cluster", None + "no-param-service", "test-cluster", input_params=None ) assert isinstance(result, RunServiceResponse) @@ -513,7 +513,7 @@ async def test_run_service_with_json_stdout_parsing(self): # Verify client method was called self.mock_client.gateway_manager.run_service.assert_called_once_with( - "json-service", "json-cluster", {"format": "json"} + "json-service", "json-cluster", input_params={"format": "json"} ) # Verify result @@ -574,7 +574,7 @@ async def test_run_service_error_response(self): # Verify client method was still called self.mock_client.gateway_manager.run_service.assert_called_once_with( - "failing-service", "test-cluster", {"timeout": 30} + "failing-service", "test-cluster", input_params={"timeout": 30} ) @pytest.mark.asyncio @@ -632,7 +632,7 @@ async def test_run_service_with_complex_parameters(self): # Verify client was called with complex parameters self.mock_client.gateway_manager.run_service.assert_called_once_with( - "complex-service", "production-cluster", complex_params + "complex-service", "production-cluster", input_params=complex_params ) assert isinstance(result, RunServiceResponse)