From ceeae84652d51e28c47ee36d63c90e9828fbf2ca Mon Sep 17 00:00:00 2001 From: David Piskula Date: Tue, 14 Oct 2025 13:03:49 +0200 Subject: [PATCH 01/12] samples: removed project config This commit removes an unncessary config for the qemu target Signed-off-by: David Piskula --- samples/net/mcp/mcp_server_hello_world/boards/qemu_x86.conf | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 samples/net/mcp/mcp_server_hello_world/boards/qemu_x86.conf diff --git a/samples/net/mcp/mcp_server_hello_world/boards/qemu_x86.conf b/samples/net/mcp/mcp_server_hello_world/boards/qemu_x86.conf deleted file mode 100644 index 9f704b23e3c27..0000000000000 --- a/samples/net/mcp/mcp_server_hello_world/boards/qemu_x86.conf +++ /dev/null @@ -1,2 +0,0 @@ -# QEMU x86 specific ethernet driver -CONFIG_ETH_E1000=y From 1ed381d719a66cf4d0becb5b654f7dbeafa73844 Mon Sep 17 00:00:00 2001 From: David Piskula Date: Tue, 14 Oct 2025 13:11:30 +0200 Subject: [PATCH 02/12] networking: thread pools and logs Added thread pool and message queue initialization and enabled logging Signed-off-by: David Piskula --- .../net/mcp/mcp_server_hello_world/prj.conf | 3 + subsys/net/lib/mcp/mcp_server.c | 89 ++++++++++++++++++- 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/samples/net/mcp/mcp_server_hello_world/prj.conf b/samples/net/mcp/mcp_server_hello_world/prj.conf index 8de80942e9609..1d1577df07705 100644 --- a/samples/net/mcp/mcp_server_hello_world/prj.conf +++ b/samples/net/mcp/mcp_server_hello_world/prj.conf @@ -17,6 +17,9 @@ CONFIG_MCP_SERVER=y CONFIG_LOG=y CONFIG_PRINTK=y +# MCP library logging level +CONFIG_MCP_LOG_LEVEL_INF=y + # Network configuration CONFIG_NET_CONFIG_SETTINGS=y CONFIG_NET_CONFIG_NEED_IPV4=y diff --git a/subsys/net/lib/mcp/mcp_server.c b/subsys/net/lib/mcp/mcp_server.c index ced87f4957a0e..f158cefd14e8e 100644 --- a/subsys/net/lib/mcp/mcp_server.c +++ b/subsys/net/lib/mcp/mcp_server.c @@ -8,8 +8,95 @@ #include #include +LOG_MODULE_REGISTER(mcp_server, CONFIG_MCP_LOG_LEVEL); + +/* Configuration defaults */ + +#define MCP_REQUEST_WORKERS 2 +#define MCP_MESSAGE_WORKERS 2 +#define MCP_REQUEST_QUEUE_SIZE 10 +#define MCP_MESSAGE_QUEUE_SIZE 10 +#define MCP_WORKER_PRIORITY 7 + +/* Message structures */ +struct mcp_request_msg { + uint32_t request_id; + /* More fields will be added later */ +}; + +struct mcp_message_msg { + uint32_t token; + /* More fields will be added later */ +}; + +/* Message queues */ +K_MSGQ_DEFINE(mcp_request_queue, sizeof(struct mcp_request_msg), MCP_REQUEST_QUEUE_SIZE, 4); +K_MSGQ_DEFINE(mcp_message_queue, sizeof(struct mcp_message_msg), MCP_MESSAGE_QUEUE_SIZE, 4); + +/* Worker thread stacks */ +K_THREAD_STACK_ARRAY_DEFINE(mcp_request_worker_stacks, MCP_REQUEST_WORKERS, 2048); +K_THREAD_STACK_ARRAY_DEFINE(mcp_message_worker_stacks, MCP_MESSAGE_WORKERS, 2048); + +/* Worker thread structures */ +static struct k_thread mcp_request_workers[MCP_REQUEST_WORKERS]; +static struct k_thread mcp_message_workers[MCP_MESSAGE_WORKERS]; + +/* Request worker function */ +static void mcp_request_worker(void *arg1, void *arg2, void *arg3) +{ + struct mcp_request_msg msg; + int worker_id = POINTER_TO_INT(arg1); + + LOG_INF("Request worker %d started", worker_id); + + while (1) { + if (k_msgq_get(&mcp_request_queue, &msg, K_FOREVER) == 0) { + LOG_DBG("Processing request (worker %d)", worker_id); + } + } +} + +/* Message worker function */ +static void mcp_message_worker(void *arg1, void *arg2, void *arg3) +{ + struct mcp_message_msg msg; + int worker_id = POINTER_TO_INT(arg1); + + LOG_INF("Message worker %d started", worker_id); + + while (1) { + if (k_msgq_get(&mcp_message_queue, &msg, K_FOREVER) == 0) { + LOG_DBG("Processing message (worker %d)", worker_id); + } + } +} + int mcp_server_init(void) { - printk("Hello World from MCP Server Core\n\r"); + LOG_INF("Initializing MCP Server Core"); + + /* Initialize request worker threads */ + for (int i = 0; i < MCP_REQUEST_WORKERS; i++) { + k_thread_create(&mcp_request_workers[i], mcp_request_worker_stacks[i], + K_THREAD_STACK_SIZEOF(mcp_request_worker_stacks[i]), + mcp_request_worker, INT_TO_POINTER(i), NULL, NULL, + K_PRIO_COOP(MCP_WORKER_PRIORITY), 0, K_NO_WAIT); + + k_thread_name_set(&mcp_request_workers[i], "mcp_req_worker"); + } + + /* Initialize message worker threads */ + for (int i = 0; i < MCP_MESSAGE_WORKERS; i++) { + k_thread_create(&mcp_message_workers[i], mcp_message_worker_stacks[i], + K_THREAD_STACK_SIZEOF(mcp_message_worker_stacks[i]), + mcp_message_worker, INT_TO_POINTER(i), NULL, NULL, + K_PRIO_COOP(MCP_WORKER_PRIORITY), 0, K_NO_WAIT); + + k_thread_name_set(&mcp_message_workers[i], "mcp_msg_worker"); + } + + LOG_INF("MCP Server Core initialized: %d request workers, %d message workers", + MCP_REQUEST_WORKERS, MCP_MESSAGE_WORKERS); + return 0; } From 43cc365023de157d63dd5add56a6c015c21393f9 Mon Sep 17 00:00:00 2001 From: David Piskula Date: Mon, 27 Oct 2025 14:13:21 +0100 Subject: [PATCH 03/12] networking: lifecycle Basic steps towards lifecycle handling, starting with initialization Signed-off-by: David Piskula --- include/zephyr/net/mcp/mcp_server.h | 18 + .../net/mcp/mcp_server_hello_world/prj.conf | 1 + .../net/mcp/mcp_server_hello_world/src/main.c | 3 + subsys/net/lib/mcp/CMakeLists.txt | 1 + subsys/net/lib/mcp/Kconfig | 151 +++++- subsys/net/lib/mcp/mcp_common.c | 19 + subsys/net/lib/mcp/mcp_common.h | 189 ++++++- subsys/net/lib/mcp/mcp_server.c | 492 +++++++++++++++++- subsys/net/lib/mcp/mcp_transport.c | 7 + subsys/net/lib/mcp/mcp_transport.h | 3 + 10 files changed, 854 insertions(+), 30 deletions(-) create mode 100644 subsys/net/lib/mcp/mcp_common.c diff --git a/include/zephyr/net/mcp/mcp_server.h b/include/zephyr/net/mcp/mcp_server.h index 92d8dc0f660b8..73cba684163d6 100644 --- a/include/zephyr/net/mcp/mcp_server.h +++ b/include/zephyr/net/mcp/mcp_server.h @@ -13,12 +13,30 @@ extern "C" { #endif +struct mcp_message_msg { + uint32_t token; + /* More fields will be added later */ +}; + /** * @brief Initialize the MCP Server. * */ int mcp_server_init(void); +/** + * @brief Start the MCP Server. + * + */ +int mcp_server_start(void); + +/** + * @brief Queues a response to the MCP Server library, which takes care of sending it to + * the MCP Client. + * + */ +int mcp_queue_response(void); + #ifdef __cplusplus } #endif diff --git a/samples/net/mcp/mcp_server_hello_world/prj.conf b/samples/net/mcp/mcp_server_hello_world/prj.conf index 1d1577df07705..491229a65b8e2 100644 --- a/samples/net/mcp/mcp_server_hello_world/prj.conf +++ b/samples/net/mcp/mcp_server_hello_world/prj.conf @@ -27,3 +27,4 @@ CONFIG_NET_CONFIG_MY_IPV4_ADDR="192.0.2.1" # Sufficient stack size for HTTP + JSON processing CONFIG_MAIN_STACK_SIZE=4096 +CONFIG_HEAP_MEM_POOL_SIZE=16384 diff --git a/samples/net/mcp/mcp_server_hello_world/src/main.c b/samples/net/mcp/mcp_server_hello_world/src/main.c index 2fb7adaf4633c..eb39d15763c59 100644 --- a/samples/net/mcp/mcp_server_hello_world/src/main.c +++ b/samples/net/mcp/mcp_server_hello_world/src/main.c @@ -13,6 +13,9 @@ LOG_MODULE_REGISTER(mcp_sample_hello, LOG_LEVEL_INF); int main(void) { printk("Hello World\n\r"); + printk("Initializing...\n\r"); mcp_server_init(); + printk("Starting...\n\r"); + mcp_server_start(); return 0; } diff --git a/subsys/net/lib/mcp/CMakeLists.txt b/subsys/net/lib/mcp/CMakeLists.txt index b2630c5beb386..2ff9d2d4cc86b 100644 --- a/subsys/net/lib/mcp/CMakeLists.txt +++ b/subsys/net/lib/mcp/CMakeLists.txt @@ -3,6 +3,7 @@ zephyr_library() +zephyr_library_sources_ifdef(CONFIG_MCP_SERVER mcp_common.c) zephyr_library_sources_ifdef(CONFIG_MCP_SERVER mcp_server.c) zephyr_library_sources_ifdef(CONFIG_MCP_SERVER mcp_transport.c) zephyr_library_sources_ifdef(CONFIG_MCP_SERVER mcp_json.c) diff --git a/subsys/net/lib/mcp/Kconfig b/subsys/net/lib/mcp/Kconfig index a0cb68c4f59d1..e474d9c89b571 100644 --- a/subsys/net/lib/mcp/Kconfig +++ b/subsys/net/lib/mcp/Kconfig @@ -5,10 +5,159 @@ menuconfig MCP_SERVER bool "Model Context Protocol (MCP) Server support" depends on NETWORKING && HTTP_SERVER && JSON_LIBRARY help - Enable support for MCP Server functionality. + Support for MCP Server functionality. if MCP_SERVER +config HTTP_SERVER_MAX_CLIENTS + int "Max number of HTTP/2 clients" + default 3 + range 1 100 + help + This setting determines the maximum number of HTTP/2 clients that the server can handle at once. + +config HTTP_SERVER_MAX_STREAMS + int "Max number of HTTP/2 streams" + default 3 + range 1 100 + help + This setting determines the maximum number of HTTP/2 streams for each client. + +config MCP_SERVER_INFO_NAME_MAX_LEN + int "Maximum server info name length" + default 64 + help + Maximum length for the server name field in serverInfo. + +config MCP_SERVER_INFO_NAME + string "Server name" + default "Zephyr MCP Server" + help + The name of the MCP server reported in serverInfo. + +config MCP_SERVER_INFO_VERSION_MAX_LEN + int "Maximum server info version length" + default 16 + help + Maximum length for the server version field in serverInfo. + +config MCP_SERVER_INFO_VERSION + string "Server version" + default "1.0.0" + help + The version of the MCP server reported in serverInfo. + +config MCP_SERVER_INFO_TITLE + bool "Include server title in serverInfo" + default n + help + Include optional title field in the serverInfo section. + +if MCP_SERVER_INFO_TITLE + +config MCP_SERVER_INFO_TITLE_MAX_LEN + int "Maximum server info title length" + default 64 + help + Maximum length for the server title field in serverInfo. + +config MCP_SERVER_INFO_TITLE_VALUE + string "Server title" + default "Zephyr MCP Server" + help + The title of the MCP server reported in serverInfo. + +endif # MCP_SERVER_INFO_TITLE + +config MCP_SERVER_INFO_INSTRUCTIONS + bool "Include server instructions in serverInfo" + default n + help + Include optional instructions field in the serverInfo section. + +if MCP_SERVER_INFO_INSTRUCTIONS + +config MCP_SERVER_INFO_INSTRUCTIONS_MAX_LEN + int "Maximum server info instructions length" + default 256 + help + Maximum length for the server instructions field in serverInfo. + +config MCP_SERVER_INFO_INSTRUCTIONS_VALUE + string "Server instructions" + default "This is a Zephyr-based MCP server providing tool capabilities." + help + The instructions for the MCP server reported in serverInfo. + +endif # MCP_SERVER_INFO_INSTRUCTIONS + +config MCP_TOOLS_CAPABILITY + bool "Tools capability" + default y + help + Tools capability in the MCP server. This allows the server + to register and execute tools via tools/list and tools/call methods. + +if MCP_TOOLS_CAPABILITY + +config MCP_MAX_TOOLS + int "Maximum number of registered tools" + default 4 + +config MCP_TOOL_NAME_MAX_LEN + int "Maximum tool name length" + default 32 + help + Maximum length for tool name and title fields. + +config MCP_TOOL_DESC + bool "Include tool description in tools/list response" + default n + help + Include optional description field in the tools/list response. + +if MCP_TOOL_DESC + +config MCP_TOOL_DESC_MAX_LEN + int "Maximum tool description length" + default 256 + help + Maximum length for tool description field. + +endif # MCP_TOOL_DESC + +config MCP_TOOL_SCHEMA_MAX_LEN + int "Maximum input/output schema length" + default 512 + help + Maximum length for tool input and output schema fields. + +config MCP_TOOL_INPUT_ARGS_MAX_LEN + int "Maximum input argument length" + default 512 + help + Maximum length for tool input arguments RPC field. + +config MCP_TOOL_TITLE + bool "Include tool title field" + default n + help + Include optional title field for tools. Adds memory overhead. + +config MCP_TOOL_OUTPUT_SCHEMA + bool "Include tool output schema field" + default n + help + Include optional output schema field for tools. Adds memory overhead. + +config MCP_TOOL_RESULT_MAX_LEN + int "Maximum tool execution result length" + default 256 + help + Maximum length for tool execution result. + +endif # MCP_TOOLS_CAPABILITY + module = MCP module-str = MCP Server source "subsys/logging/Kconfig.template.log_config" diff --git a/subsys/net/lib/mcp/mcp_common.c b/subsys/net/lib/mcp/mcp_common.c new file mode 100644 index 0000000000000..dd3cbdccb4391 --- /dev/null +++ b/subsys/net/lib/mcp/mcp_common.c @@ -0,0 +1,19 @@ +/* + * Copyright 2025 NXP + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include "mcp_common.h" + +void *mcp_alloc(size_t size) +{ + return k_malloc(size); +} + +void mcp_free(void *ptr) +{ + k_free(ptr); +} diff --git a/subsys/net/lib/mcp/mcp_common.h b/subsys/net/lib/mcp/mcp_common.h index 83f2ebcd91e9d..58d3a688b0ba1 100644 --- a/subsys/net/lib/mcp/mcp_common.h +++ b/subsys/net/lib/mcp/mcp_common.h @@ -8,6 +8,193 @@ #define ZEPHYR_SUBSYS_MCP_COMMON_H_ #include -#include + +#define MCP_MAX_REQUESTS (CONFIG_HTTP_SERVER_MAX_CLIENTS * CONFIG_HTTP_SERVER_MAX_STREAMS) + +typedef enum { + MCP_NOTIF_INITIALIZED +} mcp_notification_method_type_t; + +typedef enum { + MCP_MSG_SYSTEM, + MCP_MSG_REQUEST_INITIALIZE, +#ifdef CONFIG_MCP_TOOLS_CAPABILITY + MCP_MSG_REQUEST_TOOLS_LIST, + MCP_MSG_REQUEST_TOOLS_CALL, +#endif + MCP_MSG_RESPONSE_INITIALIZE, +#ifdef CONFIG_MCP_TOOLS_CAPABILITY + MCP_MSG_RESPONSE_TOOLS_LIST, + MCP_MSG_RESPONSE_TOOLS_CALL, +#endif + MCP_MSG_NOTIFICATION, + /* unknown method, unknown tool, other errors */ +} mcp_queue_msg_type_t; + +typedef enum { + MCP_SYS_DISCONNECTION, + MCP_SYS_CANCEL +} mcp_system_msg_type_t; + +typedef enum { + MCP_PROMPTS = 0x1, + MCP_RESOURCES = 0x2, + MCP_TOOLS = 0x4, + MCP_LOGGING = 0x8, + MCP_COMPLETION = 0x10, + MCP_PAGINATION = 0x20 +} mcp_server_capabilities_t; + +typedef enum { + MCP_EXEC_ACTIVE, + MCP_EXEC_CANCELED, + MCP_EXEC_FINISHED, + MCP_EXEC_ZOMBIE +} mcp_execution_state_t; + +#ifdef CONFIG_MCP_TOOLS_CAPABILITY +typedef struct mcp_tool_metadata { + char name[CONFIG_MCP_TOOL_NAME_MAX_LEN]; + char input_schema[CONFIG_MCP_TOOL_SCHEMA_MAX_LEN]; +#ifdef CONFIG_MCP_TOOL_DESC + char description[CONFIG_MCP_TOOL_DESC_MAX_LEN]; +#endif +#ifdef CONFIG_MCP_TOOL_TITLE + char title[CONFIG_MCP_TOOL_NAME_MAX_LEN]; +#endif +#ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA + char output_schema[CONFIG_MCP_TOOL_SCHEMA_MAX_LEN]; +#endif +} mcp_tool_metadata_t; +#endif + +typedef struct mcp_system_msg { + mcp_system_msg_type_t type; + uint32_t request_id; +} mcp_system_msg_t; + +typedef struct mcp_initialize_request { + uint32_t request_id; + uint32_t client_id; + + /* MCP Core ignores client params and clientInfo and won't try + * to use any client capabilities, in this implementation. + */ +} mcp_initialize_request_t; + +#ifdef CONFIG_MCP_TOOLS_CAPABILITY +typedef struct mcp_tools_list_request { + uint32_t request_id; + uint32_t client_id; +} mcp_tools_list_request_t; + +typedef struct mcp_tools_call_request { + uint32_t request_id; + uint32_t client_id; + + char name[CONFIG_MCP_TOOL_NAME_MAX_LEN]; + char arguments[CONFIG_MCP_TOOL_INPUT_ARGS_MAX_LEN]; +} mcp_tools_call_request_t; +#endif + +typedef struct mcp_initialize_response { + uint32_t request_id; + uint32_t capabilities; +} mcp_initialize_response_t; + +#ifdef CONFIG_MCP_TOOLS_CAPABILITY +typedef struct mcp_tools_list_response { + uint32_t request_id; + uint8_t tool_count; + mcp_tool_metadata_t tools[CONFIG_MCP_MAX_TOOLS]; +} mcp_tools_list_response_t; + +typedef struct mcp_tools_call_response { + uint32_t request_id; + char result[CONFIG_MCP_TOOL_RESULT_MAX_LEN]; +} mcp_tools_call_response_t; +#endif + +typedef struct mcp_client_notification { + uint32_t client_id; + mcp_notification_method_type_t method; +} mcp_client_notification_t; + +typedef struct mcp_server_notification { + mcp_notification_method_type_t method; +} mcp_server_notification_t; + +/* Queue message structures using void pointers */ +typedef struct mcp_request_queue_msg { + mcp_queue_msg_type_t type; + void *data; /* Points to allocated message data */ +} mcp_request_queue_msg_t; + +typedef struct mcp_response_queue_msg { + mcp_queue_msg_type_t type; + void *data; /* Points to allocated message data */ +} mcp_response_queue_msg_t; + +#ifdef CONFIG_MCP_TOOLS_CAPABILITY +/* Tool callback function signature */ +typedef int (*mcp_tool_callback_t)(const char *params, uint32_t execution_token); + +/* Tool definition structure */ +typedef struct { + char name[CONFIG_MCP_TOOL_NAME_MAX_LEN]; /* Required */ + char input_schema[CONFIG_MCP_TOOL_SCHEMA_MAX_LEN]; /* Required */ +#ifdef CONFIG_MCP_TOOL_DESC + char description[CONFIG_MCP_TOOL_DESC_MAX_LEN]; /* Optional */ +#endif +#ifdef CONFIG_MCP_TOOL_TITLE + char title[CONFIG_MCP_TOOL_NAME_MAX_LEN]; /* Optional */ +#endif +#ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA + char output_schema[CONFIG_MCP_TOOL_SCHEMA_MAX_LEN]; /* Optional */ +#endif + mcp_tool_callback_t callback; +} mcp_tool_info_t; + +/* Tool registry structure */ +typedef struct { + mcp_tool_info_t tools[CONFIG_MCP_MAX_TOOLS]; + struct k_mutex registry_mutex; + uint8_t tool_count; +} mcp_tool_registry_t; + +/* Execution context for tracking active tool executions */ +typedef struct { + uint32_t execution_token; + uint32_t request_id; + uint32_t client_id; + uint32_t worker_id; + int64_t start_timestamp; + int64_t cancel_timestamp; + int64_t last_message_timestamp; + bool worker_released; + mcp_execution_state_t execution_state; +} mcp_execution_context_t; + +/* Execution registry for tracking active executions */ +typedef struct { + mcp_execution_context_t executions[MCP_MAX_REQUESTS]; + struct k_mutex execution_mutex; +} mcp_execution_registry_t; +#endif + +/** + * @brief Allocate memory for MCP operations + * + * @param size Size in bytes to allocate + * @return Pointer to allocated memory, or NULL on failure + */ +void *mcp_alloc(size_t size); + +/** + * @brief Free memory allocated by mcp_alloc() + * + * @param ptr Pointer to memory to free, or NULL (no-op if NULL) + */ +void mcp_free(void *ptr); #endif /* ZEPHYR_SUBSYS_MCP_COMMON_H_ */ diff --git a/subsys/net/lib/mcp/mcp_server.c b/subsys/net/lib/mcp/mcp_server.c index f158cefd14e8e..315e8dfd1a2e3 100644 --- a/subsys/net/lib/mcp/mcp_server.c +++ b/subsys/net/lib/mcp/mcp_server.c @@ -7,6 +7,8 @@ #include #include #include +#include "mcp_common.h" +#include "mcp_transport.h" LOG_MODULE_REGISTER(mcp_server, CONFIG_MCP_LOG_LEVEL); @@ -18,20 +20,37 @@ LOG_MODULE_REGISTER(mcp_server, CONFIG_MCP_LOG_LEVEL); #define MCP_MESSAGE_QUEUE_SIZE 10 #define MCP_WORKER_PRIORITY 7 -/* Message structures */ -struct mcp_request_msg { - uint32_t request_id; - /* More fields will be added later */ -}; +/* MCP Client Registry structures */ -struct mcp_message_msg { - uint32_t token; - /* More fields will be added later */ -}; +/* MCP Lifecycle states for client connections */ +typedef enum { + MCP_LIFECYCLE_NEW, + MCP_LIFECYCLE_INITIALIZING, + MCP_LIFECYCLE_INITIALIZED, + MCP_LIFECYCLE_DEINITIALIZING, +} mcp_lifecycle_state_t; + +/* Client connection context */ +typedef struct { + uint32_t client_id; + mcp_lifecycle_state_t lifecycle_state; + uint32_t active_requests[MCP_MAX_REQUESTS]; + uint8_t active_request_count; +} mcp_client_context_t; + +/* MCP Client Registry */ +typedef struct { + mcp_client_context_t clients[CONFIG_HTTP_SERVER_MAX_CLIENTS]; + struct k_mutex registry_mutex; +} mcp_client_registry_t; + +/* Global client registry instance */ +static mcp_client_registry_t client_registry; +static mcp_tool_registry_t tool_registry; /* Message queues */ -K_MSGQ_DEFINE(mcp_request_queue, sizeof(struct mcp_request_msg), MCP_REQUEST_QUEUE_SIZE, 4); -K_MSGQ_DEFINE(mcp_message_queue, sizeof(struct mcp_message_msg), MCP_MESSAGE_QUEUE_SIZE, 4); +K_MSGQ_DEFINE(mcp_request_queue, sizeof(mcp_request_queue_msg_t), MCP_REQUEST_QUEUE_SIZE, 4); +K_MSGQ_DEFINE(mcp_message_queue, sizeof(mcp_response_queue_msg_t), MCP_MESSAGE_QUEUE_SIZE, 4); /* Worker thread stacks */ K_THREAD_STACK_ARRAY_DEFINE(mcp_request_worker_stacks, MCP_REQUEST_WORKERS, 2048); @@ -44,14 +63,377 @@ static struct k_thread mcp_message_workers[MCP_MESSAGE_WORKERS]; /* Request worker function */ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) { - struct mcp_request_msg msg; + mcp_request_queue_msg_t request; + bool client_registered = false; + int client_index = -1; int worker_id = POINTER_TO_INT(arg1); + int ret; LOG_INF("Request worker %d started", worker_id); while (1) { - if (k_msgq_get(&mcp_request_queue, &msg, K_FOREVER) == 0) { - LOG_DBG("Processing request (worker %d)", worker_id); + ret = k_msgq_get(&mcp_request_queue, &request, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to get request from queue: %d", ret); + continue; + } + + LOG_DBG("Processing request (worker %d)", worker_id); + + if (request.data == NULL) { + LOG_ERR("Received NULL data pointer in request"); + continue; + } + switch (request.type) { + case MCP_MSG_SYSTEM: { + mcp_system_msg_t *sys_msg; + + LOG_DBG("Processing system request"); + sys_msg = (mcp_system_msg_t *)request.data; + /* Add system request handling logic */ + mcp_free(request.data); + } break; + + case MCP_MSG_REQUEST_INITIALIZE: { + mcp_initialize_request_t *rpc_request; + mcp_initialize_response_t *response; + mcp_response_queue_msg_t queue_response; + + LOG_DBG("Processing RPC request"); + rpc_request = (mcp_initialize_request_t *)request.data; + + /* Reset client tracking variables */ + client_registered = false; + client_index = -1; + + /* Add RPC request handling logic */ + ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock client registry mutex: %d", ret); + mcp_free(rpc_request); + break; + } + + /* Search for existing client */ + for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { + if (client_registry.clients[i].client_id == + rpc_request->client_id) { + client_registered = true; + client_index = i; + LOG_DBG("Found client with ID %u", rpc_request->client_id); + break; + } + } + + /* Register new client if not found */ + if (!client_registered) { + for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { + if (client_registry.clients[i].client_id == 0) { + /* Found an empty slot in the client registry */ + client_registry.clients[i].client_id = + rpc_request->client_id; + client_registry.clients[i].lifecycle_state = + MCP_LIFECYCLE_NEW; + client_registry.clients[i].active_request_count = 0; + + client_index = i; + LOG_DBG("Registered new client with ID %u", + rpc_request->client_id); + break; + } + } + + if (client_index == -1) { + LOG_ERR("Client registry is full. Cannot register new " + "client."); + k_mutex_unlock(&client_registry.registry_mutex); + mcp_free(rpc_request); + break; + } + } + + if (client_registry.clients[client_index].lifecycle_state == + MCP_LIFECYCLE_NEW) { + client_registry.clients[client_index].lifecycle_state = + MCP_LIFECYCLE_INITIALIZING; + LOG_DBG("Client %u initializing", rpc_request->client_id); + } else { + LOG_ERR("Client %u already initialized", rpc_request->client_id); + k_mutex_unlock(&client_registry.registry_mutex); + mcp_free(rpc_request); + break; + } + k_mutex_unlock(&client_registry.registry_mutex); + + response = (mcp_initialize_response_t *)mcp_alloc( + sizeof(mcp_initialize_response_t)); + if (!response) { + LOG_ERR("Failed to allocate response for client %u", + rpc_request->client_id); + /* TODO: Queue error response */ + break; + } + + response->request_id = rpc_request->request_id; + + /* Set capabilities based on enabled features */ + response->capabilities = 0; +#ifdef CONFIG_MCP_TOOLS_CAPABILITY + response->capabilities |= MCP_TOOLS; +#endif + + queue_response.type = MCP_MSG_RESPONSE_INITIALIZE; + queue_response.data = response; + + ret = mcp_transport_queue_response(&queue_response); + if (ret != 0) { + LOG_ERR("Failed to queue response: %d", ret); + mcp_free(response); + } + + mcp_free(rpc_request); + break; + } + +#ifdef CONFIG_MCP_TOOLS_CAPABILITY + case MCP_MSG_REQUEST_TOOLS_LIST: { + mcp_tools_list_request_t *rpc_request; + mcp_tools_list_response_t *response; + mcp_response_queue_msg_t queue_response; + + LOG_DBG("Processing RPC request"); + rpc_request = (mcp_tools_list_request_t *)request.data; + + /* Reset client tracking variables */ + client_registered = false; + + /* Add RPC request handling logic */ + ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock client registry mutex: %d", ret); + mcp_free(rpc_request); + break; + } + + /* Search for existing client */ + for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { + if (client_registry.clients[i].client_id == + rpc_request->client_id) { + client_registered = true; + + if (client_registry.clients[i].lifecycle_state == + MCP_LIFECYCLE_INITIALIZED) { + LOG_DBG("Client %u initialized, processing request", + rpc_request->client_id); + } else { + LOG_ERR("Client %u not initialized. Refusing to " + "process tools list request.", + rpc_request->client_id); + k_mutex_unlock(&client_registry.registry_mutex); + mcp_free(rpc_request); + break; + } + } + } + + if (client_registered == false) { + LOG_ERR("Client not registered. Refusing to process tools list " + "request."); + /* TODO: Handle error response */ + k_mutex_unlock(&client_registry.registry_mutex); + mcp_free(rpc_request); + break; + } + + k_mutex_unlock(&client_registry.registry_mutex); + + LOG_DBG("Retrieving tools list for client %u", rpc_request->client_id); + response = (mcp_tools_list_response_t *)mcp_alloc( + sizeof(mcp_tools_list_response_t)); + if (!response) { + LOG_ERR("Failed to allocate tools list response for client %u", + rpc_request->client_id); + mcp_free(rpc_request); + /* TODO: Queue error response */ + break; + } + + response->request_id = rpc_request->request_id; + /* Lock tool registry and copy tool information */ + ret = k_mutex_lock(&tool_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock tool registry mutex: %d", ret); + mcp_free(rpc_request); + mcp_free(response); + break; + } + + response->tool_count = tool_registry.tool_count; + /* Copy tool metadata for transport layer */ + for (int i = 0; i < tool_registry.tool_count; i++) { + /* Required fields - always copy */ + strncpy(response->tools[i].name, tool_registry.tools[i].name, + CONFIG_MCP_TOOL_NAME_MAX_LEN - 1); + response->tools[i].name[CONFIG_MCP_TOOL_NAME_MAX_LEN - 1] = '\0'; + + strncpy(response->tools[i].input_schema, + tool_registry.tools[i].input_schema, + CONFIG_MCP_TOOL_SCHEMA_MAX_LEN - 1); + response->tools[i] + .input_schema[CONFIG_MCP_TOOL_SCHEMA_MAX_LEN - 1] = '\0'; + +#ifdef CONFIG_MCP_TOOL_DESC + /* Description - only copy if not empty */ + if (strlen(tool_registry.tools[i].description) > 0) { + strncpy(response->tools[i].description, + tool_registry.tools[i].description, + CONFIG_MCP_TOOL_DESC_MAX_LEN - 1); + response->tools[i] + .description[CONFIG_MCP_TOOL_DESC_MAX_LEN - 1] = + '\0'; + } else { + response->tools[i].description[0] = '\0'; + } +#endif + +#ifdef CONFIG_MCP_TOOL_TITLE + /* Title - only copy if not empty */ + if (strlen(tool_registry.tools[i].title) > 0) { + strncpy(response->tools[i].title, + tool_registry.tools[i].title, + CONFIG_MCP_TOOL_NAME_MAX_LEN - 1); + response->tools[i].title[CONFIG_MCP_TOOL_NAME_MAX_LEN - 1] = + '\0'; + } else { + response->tools[i].title[0] = '\0'; + } +#endif + +#ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA + /* Output schema - only copy if not empty */ + if (strlen(tool_registry.tools[i].output_schema) > 0) { + strncpy(response->tools[i].output_schema, + tool_registry.tools[i].output_schema, + CONFIG_MCP_TOOL_SCHEMA_MAX_LEN - 1); + response->tools[i] + .output_schema[CONFIG_MCP_TOOL_SCHEMA_MAX_LEN - 1] = + '\0'; + } else { + response->tools[i].output_schema[0] = '\0'; + } +#endif + } + + k_mutex_unlock(&tool_registry.registry_mutex); + + queue_response.type = MCP_MSG_RESPONSE_TOOLS_LIST; + queue_response.data = response; + + ret = mcp_transport_queue_response(&queue_response); + if (ret != 0) { + LOG_ERR("Failed to queue tools list response: %d", ret); + mcp_free(rpc_request); + mcp_free(response); + break; + } + + mcp_free(rpc_request); + break; + } + + case MCP_MSG_REQUEST_TOOLS_CALL: { + mcp_tools_call_request_t *rpc_request; + mcp_tools_call_response_t *response; + mcp_response_queue_msg_t queue_response; + + rpc_request = (mcp_tools_call_request_t *)request.data; + LOG_DBG("Calling tool for client %u", rpc_request->client_id); + /* TODO: Implement tool call handling */ + + mcp_free(rpc_request); + break; + } +#endif + case MCP_MSG_NOTIFICATION: { + mcp_client_notification_t *notification; + + LOG_DBG("Processing RPC notification"); + notification = (mcp_client_notification_t *)request.data; + + /* Reset client tracking variables */ + client_registered = false; + client_index = -1; + + /* Add RPC notification handling logic */ + ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock client registry mutex: %d", ret); + mcp_free(notification); + break; + } + + /* Search for client */ + for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { + if (client_registry.clients[i].client_id == + notification->client_id) { + client_registered = true; + client_index = i; + LOG_DBG("Found client with ID %u", notification->client_id); + break; + } + } + k_mutex_unlock(&client_registry.registry_mutex); + + if (client_registered == false) { + LOG_ERR("Client not registered. Refusing to process notification."); + /* TODO: Handle error response */ + mcp_free(notification); + break; + } + + switch (notification->method) { + case MCP_NOTIF_INITIALIZED: { + ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock client registry mutex: %d", ret); + break; + } + + if (client_registry.clients[client_index].lifecycle_state == + MCP_LIFECYCLE_INITIALIZING) { + client_registry.clients[client_index].lifecycle_state = + MCP_LIFECYCLE_INITIALIZED; + LOG_DBG("Client %u initialization complete", + notification->client_id); + } else { + LOG_ERR("Invalid state transition for client %u", + notification->client_id); + /* TODO: Respond with an error message */ + k_mutex_unlock(&client_registry.registry_mutex); + break; + } + k_mutex_unlock(&client_registry.registry_mutex); + break; + } + + default: { + LOG_ERR("Unknown notification method %u", notification->method); + /* TODO: Respond with an error message */ + break; + } + } + + mcp_free(notification); + break; + } + + default: { + LOG_ERR("Unknown message type %u", request.type); + if (request.data) { + mcp_free(request.data); + } + break; + } } } } @@ -61,41 +443,95 @@ static void mcp_message_worker(void *arg1, void *arg2, void *arg3) { struct mcp_message_msg msg; int worker_id = POINTER_TO_INT(arg1); + int ret; LOG_INF("Message worker %d started", worker_id); while (1) { - if (k_msgq_get(&mcp_message_queue, &msg, K_FOREVER) == 0) { + ret = k_msgq_get(&mcp_message_queue, &msg, K_FOREVER); + if (ret == 0) { LOG_DBG("Processing message (worker %d)", worker_id); + /* TODO: Implement message processing */ + } else { + LOG_ERR("Failed to get message from queue: %d", ret); } } } int mcp_server_init(void) { + int ret; + LOG_INF("Initializing MCP Server Core"); - /* Initialize request worker threads */ + /* Initialize client registry mutex */ + ret = k_mutex_init(&client_registry.registry_mutex); + if (ret != 0) { + LOG_ERR("Failed to initialize client registry mutex: %d", ret); + return ret; + } + + /* Initialize tool registry mutex */ + ret = k_mutex_init(&tool_registry.registry_mutex); + if (ret != 0) { + LOG_ERR("Failed to initialize tool registry mutex: %d", ret); + return ret; + } + + /* Initialize client registry */ + memset(&client_registry.clients, 0, sizeof(client_registry.clients)); + + /* Initialize tool registry */ + tool_registry.tool_count = 0; + memset(&tool_registry.tools, 0, sizeof(tool_registry.tools)); + + LOG_INF("MCP Server Core initialized"); + + return 0; +} + +int mcp_server_start(void) +{ + k_tid_t tid; + int ret; + + LOG_INF("Starting MCP Server Core"); + + /* Create and start request worker threads */ for (int i = 0; i < MCP_REQUEST_WORKERS; i++) { - k_thread_create(&mcp_request_workers[i], mcp_request_worker_stacks[i], - K_THREAD_STACK_SIZEOF(mcp_request_worker_stacks[i]), - mcp_request_worker, INT_TO_POINTER(i), NULL, NULL, - K_PRIO_COOP(MCP_WORKER_PRIORITY), 0, K_NO_WAIT); + tid = k_thread_create(&mcp_request_workers[i], mcp_request_worker_stacks[i], + K_THREAD_STACK_SIZEOF(mcp_request_worker_stacks[i]), + mcp_request_worker, INT_TO_POINTER(i), NULL, NULL, + K_PRIO_COOP(MCP_WORKER_PRIORITY), 0, K_NO_WAIT); + if (tid == NULL) { + LOG_ERR("Failed to create request worker thread %d", i); + return -ENOMEM; + } - k_thread_name_set(&mcp_request_workers[i], "mcp_req_worker"); + ret = k_thread_name_set(&mcp_request_workers[i], "mcp_req_worker"); + if (ret != 0) { + LOG_WRN("Failed to set name for request worker thread %d: %d", i, ret); + } } - /* Initialize message worker threads */ + /* Create and start message worker threads */ for (int i = 0; i < MCP_MESSAGE_WORKERS; i++) { - k_thread_create(&mcp_message_workers[i], mcp_message_worker_stacks[i], - K_THREAD_STACK_SIZEOF(mcp_message_worker_stacks[i]), - mcp_message_worker, INT_TO_POINTER(i), NULL, NULL, - K_PRIO_COOP(MCP_WORKER_PRIORITY), 0, K_NO_WAIT); + tid = k_thread_create(&mcp_message_workers[i], mcp_message_worker_stacks[i], + K_THREAD_STACK_SIZEOF(mcp_message_worker_stacks[i]), + mcp_message_worker, INT_TO_POINTER(i), NULL, NULL, + K_PRIO_COOP(MCP_WORKER_PRIORITY), 0, K_NO_WAIT); + if (tid == NULL) { + LOG_ERR("Failed to create message worker thread %d", i); + return -ENOMEM; + } - k_thread_name_set(&mcp_message_workers[i], "mcp_msg_worker"); + ret = k_thread_name_set(&mcp_message_workers[i], "mcp_msg_worker"); + if (ret != 0) { + LOG_WRN("Failed to set name for message worker thread %d: %d", i, ret); + } } - LOG_INF("MCP Server Core initialized: %d request workers, %d message workers", + LOG_INF("MCP Server Core started: %d request workers, %d message workers", MCP_REQUEST_WORKERS, MCP_MESSAGE_WORKERS); return 0; diff --git a/subsys/net/lib/mcp/mcp_transport.c b/subsys/net/lib/mcp/mcp_transport.c index 0c1fb7a3a7474..b0b132a431b04 100644 --- a/subsys/net/lib/mcp/mcp_transport.c +++ b/subsys/net/lib/mcp/mcp_transport.c @@ -7,3 +7,10 @@ #include #include #include +#include "mcp_transport.h" + +int mcp_transport_queue_response(mcp_response_queue_msg_t *msg) +{ + /* queue msg to the correct requests queue */ + return 0; +} diff --git a/subsys/net/lib/mcp/mcp_transport.h b/subsys/net/lib/mcp/mcp_transport.h index abf22483abbd0..b361544589899 100644 --- a/subsys/net/lib/mcp/mcp_transport.h +++ b/subsys/net/lib/mcp/mcp_transport.h @@ -9,5 +9,8 @@ #include #include +#include "mcp_common.h" + +int mcp_transport_queue_response(mcp_response_queue_msg_t *msg); #endif /* ZEPHYR_SUBSYS_MCP_TRANSPORT_H_ */ From 6db6b53028883f430c149c66d40aca223d959e71 Mon Sep 17 00:00:00 2001 From: David Piskula Date: Mon, 3 Nov 2025 14:11:42 +0100 Subject: [PATCH 04/12] networking: tools Added tool registration, improved type definitions and tools/list Signed-off-by: David Piskula --- include/zephyr/net/mcp/mcp_server.h | 52 +++++++ .../net/mcp/mcp_server_hello_world/src/main.c | 91 ++++++++++- subsys/net/lib/mcp/mcp_common.h | 38 +---- subsys/net/lib/mcp/mcp_server.c | 142 +++++++++++++++--- 4 files changed, 267 insertions(+), 56 deletions(-) diff --git a/include/zephyr/net/mcp/mcp_server.h b/include/zephyr/net/mcp/mcp_server.h index 73cba684163d6..a209f0a966715 100644 --- a/include/zephyr/net/mcp/mcp_server.h +++ b/include/zephyr/net/mcp/mcp_server.h @@ -8,11 +8,37 @@ #define ZEPHYR_INCLUDE_NET_MCP_SERVER_H_ #include +#include #ifdef __cplusplus extern "C" { #endif +#ifdef CONFIG_MCP_TOOLS_CAPABILITY +typedef struct mcp_tool_metadata { + char name[CONFIG_MCP_TOOL_NAME_MAX_LEN]; + char input_schema[CONFIG_MCP_TOOL_SCHEMA_MAX_LEN]; +#ifdef CONFIG_MCP_TOOL_DESC + char description[CONFIG_MCP_TOOL_DESC_MAX_LEN]; +#endif +#ifdef CONFIG_MCP_TOOL_TITLE + char title[CONFIG_MCP_TOOL_NAME_MAX_LEN]; +#endif +#ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA + char output_schema[CONFIG_MCP_TOOL_SCHEMA_MAX_LEN]; +#endif +} mcp_tool_metadata_t; + +/* Tool callback function signature */ +typedef int (*mcp_tool_callback_t)(const char *params, uint32_t execution_token); + +/* Tool definition structure */ +typedef struct mcp_tool_record { + mcp_tool_metadata_t metadata; + mcp_tool_callback_t callback; +} mcp_tool_record_t; +#endif + struct mcp_message_msg { uint32_t token; /* More fields will be added later */ @@ -21,12 +47,14 @@ struct mcp_message_msg { /** * @brief Initialize the MCP Server. * + * @return 0 on success, negative errno code on failure */ int mcp_server_init(void); /** * @brief Start the MCP Server. * + * @return 0 on success, negative errno code on failure */ int mcp_server_start(void); @@ -34,9 +62,33 @@ int mcp_server_start(void); * @brief Queues a response to the MCP Server library, which takes care of sending it to * the MCP Client. * + * @return 0 on success, negative errno code on failure */ int mcp_queue_response(void); +#ifdef CONFIG_MCP_TOOLS_CAPABILITY +/** + * @brief Add a tool to the MCP server tool registry + * + * @param tool_record Pointer to tool record containing metadata and callback + * @return 0 on success, negative errno code on failure + * -EINVAL if tool_record is NULL or invalid + * -EEXIST if tool with same name already exists + * -ENOSPC if tool registry is full + */ +int mcp_server_add_tool(const mcp_tool_record_t *tool_record); + +/** + * @brief Remove a tool from the MCP server tool registry + * + * @param tool_name Name of the tool to remove + * @return 0 on success, negative errno code on failure + * -EINVAL if tool_name is NULL + * -ENOENT if tool not found + */ +int mcp_server_remove_tool(const char *tool_name); +#endif + #ifdef __cplusplus } #endif diff --git a/samples/net/mcp/mcp_server_hello_world/src/main.c b/samples/net/mcp/mcp_server_hello_world/src/main.c index eb39d15763c59..2647a7b22ee79 100644 --- a/samples/net/mcp/mcp_server_hello_world/src/main.c +++ b/samples/net/mcp/mcp_server_hello_world/src/main.c @@ -10,12 +10,99 @@ LOG_MODULE_REGISTER(mcp_sample_hello, LOG_LEVEL_INF); +#ifdef CONFIG_MCP_TOOLS_CAPABILITY +/* Tool callback functions */ +static int hello_world_tool_callback(const char *params, uint32_t execution_token) +{ + printk("Hello World tool executed with params: %s, token: %u\n", params ? params : "none", + execution_token); + return 0; +} + +static int goodbye_world_tool_callback(const char *params, uint32_t execution_token) +{ + printk("Goodbye World tool executed with params: %s, token: %u\n", params ? params : "none", + execution_token); + return 0; +} + +/* Tool definitions */ +static const mcp_tool_record_t hello_world_tool = { + .metadata = { + .name = "hello_world", + .input_schema = "{\"type\":\"object\",\"properties\":{\"message\":{" + "\"type\":\"string\"}}}", +#ifdef CONFIG_MCP_TOOL_DESC + .description = "A simple hello world greeting tool", +#endif +#ifdef CONFIG_MCP_TOOL_TITLE + .title = "Hello World Tool", +#endif +#ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA + .output_schema = "{\"type\":\"object\",\"properties\":{\"response\":{" + "\"type\":\"string\"}}}", +#endif + }, + .callback = hello_world_tool_callback}; + +static const mcp_tool_record_t goodbye_world_tool = { + .metadata = { + .name = "goodbye_world", + .input_schema = "{\"type\":\"object\",\"properties\":{\"message\":{" + "\"type\":\"string\"}}}", +#ifdef CONFIG_MCP_TOOL_DESC + .description = "A simple goodbye world farewell tool", +#endif +#ifdef CONFIG_MCP_TOOL_TITLE + .title = "Goodbye World Tool", +#endif +#ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA + .output_schema = "{\"type\":\"object\",\"properties\":{\"response\":{" + "\"type\":\"string\"}}}", +#endif + }, + .callback = goodbye_world_tool_callback}; +#endif /* CONFIG_MCP_TOOLS_CAPABILITY */ + int main(void) { + int ret; + printk("Hello World\n\r"); printk("Initializing...\n\r"); - mcp_server_init(); + ret = mcp_server_init(); + if (ret != 0) { + printk("MCP Server initialization failed: %d\n\r", ret); + return ret; + } + +#ifdef CONFIG_MCP_TOOLS_CAPABILITY + printk("Registering Tool #1: Hello world!...\n\r"); + ret = mcp_server_add_tool(&hello_world_tool); + if (ret != 0) { + printk("Tool #1 registration failed.\n\r"); + return ret; + } + printk("Tool #1 registered.\n\r"); + + printk("Registering Tool #2: Goodbye world!...\n\r"); + ret = mcp_server_add_tool(&goodbye_world_tool); + if (ret != 0) { + printk("Tool #2 registration failed.\n\r"); + return ret; + } + printk("Tool #2 registered.\n\r"); +#else + printk("MCP Tools capability not enabled - skipping tool registration\n\r"); +#endif /* CONFIG_MCP_TOOLS_CAPABILITY */ + printk("Starting...\n\r"); - mcp_server_start(); + ret = mcp_server_start(); + if (ret != 0) { + printk("MCP Server start failed: %d\n\r", ret); + return ret; + } + + printk("MCP Server running...\n\r"); return 0; } diff --git a/subsys/net/lib/mcp/mcp_common.h b/subsys/net/lib/mcp/mcp_common.h index 58d3a688b0ba1..916129c32085e 100644 --- a/subsys/net/lib/mcp/mcp_common.h +++ b/subsys/net/lib/mcp/mcp_common.h @@ -8,6 +8,7 @@ #define ZEPHYR_SUBSYS_MCP_COMMON_H_ #include +#include #define MCP_MAX_REQUESTS (CONFIG_HTTP_SERVER_MAX_CLIENTS * CONFIG_HTTP_SERVER_MAX_STREAMS) @@ -52,22 +53,6 @@ typedef enum { MCP_EXEC_ZOMBIE } mcp_execution_state_t; -#ifdef CONFIG_MCP_TOOLS_CAPABILITY -typedef struct mcp_tool_metadata { - char name[CONFIG_MCP_TOOL_NAME_MAX_LEN]; - char input_schema[CONFIG_MCP_TOOL_SCHEMA_MAX_LEN]; -#ifdef CONFIG_MCP_TOOL_DESC - char description[CONFIG_MCP_TOOL_DESC_MAX_LEN]; -#endif -#ifdef CONFIG_MCP_TOOL_TITLE - char title[CONFIG_MCP_TOOL_NAME_MAX_LEN]; -#endif -#ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA - char output_schema[CONFIG_MCP_TOOL_SCHEMA_MAX_LEN]; -#endif -} mcp_tool_metadata_t; -#endif - typedef struct mcp_system_msg { mcp_system_msg_type_t type; uint32_t request_id; @@ -136,28 +121,9 @@ typedef struct mcp_response_queue_msg { } mcp_response_queue_msg_t; #ifdef CONFIG_MCP_TOOLS_CAPABILITY -/* Tool callback function signature */ -typedef int (*mcp_tool_callback_t)(const char *params, uint32_t execution_token); - -/* Tool definition structure */ -typedef struct { - char name[CONFIG_MCP_TOOL_NAME_MAX_LEN]; /* Required */ - char input_schema[CONFIG_MCP_TOOL_SCHEMA_MAX_LEN]; /* Required */ -#ifdef CONFIG_MCP_TOOL_DESC - char description[CONFIG_MCP_TOOL_DESC_MAX_LEN]; /* Optional */ -#endif -#ifdef CONFIG_MCP_TOOL_TITLE - char title[CONFIG_MCP_TOOL_NAME_MAX_LEN]; /* Optional */ -#endif -#ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA - char output_schema[CONFIG_MCP_TOOL_SCHEMA_MAX_LEN]; /* Optional */ -#endif - mcp_tool_callback_t callback; -} mcp_tool_info_t; - /* Tool registry structure */ typedef struct { - mcp_tool_info_t tools[CONFIG_MCP_MAX_TOOLS]; + mcp_tool_record_t tools[CONFIG_MCP_MAX_TOOLS]; struct k_mutex registry_mutex; uint8_t tool_count; } mcp_tool_registry_t; diff --git a/subsys/net/lib/mcp/mcp_server.c b/subsys/net/lib/mcp/mcp_server.c index 315e8dfd1a2e3..fb1e1058b4cd3 100644 --- a/subsys/net/lib/mcp/mcp_server.c +++ b/subsys/net/lib/mcp/mcp_server.c @@ -7,6 +7,7 @@ #include #include #include +#include #include "mcp_common.h" #include "mcp_transport.h" @@ -65,6 +66,7 @@ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) { mcp_request_queue_msg_t request; bool client_registered = false; + bool client_initialized = false; int client_index = -1; int worker_id = POINTER_TO_INT(arg1); int ret; @@ -171,6 +173,7 @@ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) LOG_ERR("Failed to allocate response for client %u", rpc_request->client_id); /* TODO: Queue error response */ + mcp_free(rpc_request); break; } @@ -206,6 +209,7 @@ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) /* Reset client tracking variables */ client_registered = false; + client_initialized = false; /* Add RPC request handling logic */ ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); @@ -225,18 +229,14 @@ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) MCP_LIFECYCLE_INITIALIZED) { LOG_DBG("Client %u initialized, processing request", rpc_request->client_id); - } else { - LOG_ERR("Client %u not initialized. Refusing to " - "process tools list request.", - rpc_request->client_id); - k_mutex_unlock(&client_registry.registry_mutex); - mcp_free(rpc_request); - break; + + client_initialized = true; } + break; } } - if (client_registered == false) { + if (!client_registered) { LOG_ERR("Client not registered. Refusing to process tools list " "request."); /* TODO: Handle error response */ @@ -245,6 +245,15 @@ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) break; } + if (!client_initialized) { + LOG_ERR("Client not initialized. Refusing to process tools list " + "request."); + /* TODO: Handle error response */ + k_mutex_unlock(&client_registry.registry_mutex); + mcp_free(rpc_request); + break; + } + k_mutex_unlock(&client_registry.registry_mutex); LOG_DBG("Retrieving tools list for client %u", rpc_request->client_id); @@ -272,21 +281,22 @@ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) /* Copy tool metadata for transport layer */ for (int i = 0; i < tool_registry.tool_count; i++) { /* Required fields - always copy */ - strncpy(response->tools[i].name, tool_registry.tools[i].name, + strncpy(response->tools[i].name, + tool_registry.tools[i].metadata.name, CONFIG_MCP_TOOL_NAME_MAX_LEN - 1); response->tools[i].name[CONFIG_MCP_TOOL_NAME_MAX_LEN - 1] = '\0'; strncpy(response->tools[i].input_schema, - tool_registry.tools[i].input_schema, + tool_registry.tools[i].metadata.input_schema, CONFIG_MCP_TOOL_SCHEMA_MAX_LEN - 1); response->tools[i] .input_schema[CONFIG_MCP_TOOL_SCHEMA_MAX_LEN - 1] = '\0'; #ifdef CONFIG_MCP_TOOL_DESC /* Description - only copy if not empty */ - if (strlen(tool_registry.tools[i].description) > 0) { + if (strlen(tool_registry.tools[i].metadata.description) > 0) { strncpy(response->tools[i].description, - tool_registry.tools[i].description, + tool_registry.tools[i].metadata.description, CONFIG_MCP_TOOL_DESC_MAX_LEN - 1); response->tools[i] .description[CONFIG_MCP_TOOL_DESC_MAX_LEN - 1] = @@ -298,9 +308,9 @@ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) #ifdef CONFIG_MCP_TOOL_TITLE /* Title - only copy if not empty */ - if (strlen(tool_registry.tools[i].title) > 0) { + if (strlen(tool_registry.tools[i].metadata.title) > 0) { strncpy(response->tools[i].title, - tool_registry.tools[i].title, + tool_registry.tools[i].metadata.title, CONFIG_MCP_TOOL_NAME_MAX_LEN - 1); response->tools[i].title[CONFIG_MCP_TOOL_NAME_MAX_LEN - 1] = '\0'; @@ -311,9 +321,9 @@ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) #ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA /* Output schema - only copy if not empty */ - if (strlen(tool_registry.tools[i].output_schema) > 0) { + if (strlen(tool_registry.tools[i].metadata.output_schema) > 0) { strncpy(response->tools[i].output_schema, - tool_registry.tools[i].output_schema, + tool_registry.tools[i].metadata.output_schema, CONFIG_MCP_TOOL_SCHEMA_MAX_LEN - 1); response->tools[i] .output_schema[CONFIG_MCP_TOOL_SCHEMA_MAX_LEN - 1] = @@ -382,15 +392,17 @@ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) break; } } - k_mutex_unlock(&client_registry.registry_mutex); - if (client_registered == false) { + if (!client_registered) { LOG_ERR("Client not registered. Refusing to process notification."); /* TODO: Handle error response */ + k_mutex_unlock(&client_registry.registry_mutex); mcp_free(notification); break; } + k_mutex_unlock(&client_registry.registry_mutex); + switch (notification->method) { case MCP_NOTIF_INITIALIZED: { ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); @@ -536,3 +548,97 @@ int mcp_server_start(void) return 0; } + +#ifdef CONFIG_MCP_TOOLS_CAPABILITY +int mcp_server_add_tool(const mcp_tool_record_t *tool_record) +{ + int ret; + int available_slot = -1; + + if (!tool_record || !tool_record->metadata.name[0] || !tool_record->callback) { + LOG_ERR("Invalid tool info provided"); + return -EINVAL; + } + + ret = k_mutex_lock(&tool_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock tool registry mutex: %d", ret); + return ret; + } + + /* Check for duplicate names and find first available slot */ + for (int i = 0; i < CONFIG_MCP_MAX_TOOLS; i++) { + /* Check if slot is empty (available) */ + if (tool_registry.tools[i].metadata.name[0] == '\0' && available_slot == -1) { + available_slot = i; + } + + /* Check for duplicate tool name */ + if (tool_registry.tools[i].metadata.name[0] != '\0' && + strncmp(tool_registry.tools[i].metadata.name, tool_record->metadata.name, + CONFIG_MCP_TOOL_NAME_MAX_LEN) == 0) { + LOG_ERR("Tool with name '%s' already exists", tool_record->metadata.name); + k_mutex_unlock(&tool_registry.registry_mutex); + return -EEXIST; + } + } + + /* Check if registry is full */ + if (available_slot == -1) { + LOG_ERR("Tool registry is full. Cannot add tool '%s'", tool_record->metadata.name); + k_mutex_unlock(&tool_registry.registry_mutex); + return -ENOSPC; + } + + /* Copy the tool record to the registry */ + memcpy(&tool_registry.tools[available_slot], tool_record, sizeof(mcp_tool_record_t)); + tool_registry.tool_count++; + + LOG_INF("Tool '%s' registered successfully at slot %d", tool_record->metadata.name, + available_slot); + + k_mutex_unlock(&tool_registry.registry_mutex); + + return 0; +} + +int mcp_server_remove_tool(const char *tool_name) +{ + int ret; + bool found = false; + + if (!tool_name || !tool_name[0]) { + LOG_ERR("Invalid tool name provided"); + return -EINVAL; + } + + ret = k_mutex_lock(&tool_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock tool registry mutex: %d", ret); + return ret; + } + + /* Find and remove the tool */ + for (int i = 0; i < CONFIG_MCP_MAX_TOOLS; i++) { + if (tool_registry.tools[i].metadata.name[0] != '\0' && + strncmp(tool_registry.tools[i].metadata.name, tool_name, + CONFIG_MCP_TOOL_NAME_MAX_LEN) == 0) { + /* Clear the tool slot */ + memset(&tool_registry.tools[i], 0, sizeof(mcp_tool_record_t)); + tool_registry.tool_count--; + found = true; + LOG_INF("Tool '%s' removed successfully", tool_name); + break; + } + } + + k_mutex_unlock(&tool_registry.registry_mutex); + + if (!found) { + LOG_ERR("Tool '%s' not found", tool_name); + return -ENOENT; + } + + return 0; +} +#endif From 659959d2c73058a1072db5c126c5960db1bffd08 Mon Sep 17 00:00:00 2001 From: David Piskula Date: Tue, 4 Nov 2025 14:36:45 +0100 Subject: [PATCH 05/12] networking: refactorization Refactored comments and code Signed-off-by: David Piskula --- include/zephyr/net/mcp/mcp_server.h | 56 ++- subsys/net/lib/mcp/mcp_common.h | 29 +- subsys/net/lib/mcp/mcp_server.c | 717 ++++++++++++++-------------- 3 files changed, 392 insertions(+), 410 deletions(-) diff --git a/include/zephyr/net/mcp/mcp_server.h b/include/zephyr/net/mcp/mcp_server.h index a209f0a966715..231ac1a24fca4 100644 --- a/include/zephyr/net/mcp/mcp_server.h +++ b/include/zephyr/net/mcp/mcp_server.h @@ -7,6 +7,11 @@ #ifndef ZEPHYR_INCLUDE_NET_MCP_SERVER_H_ #define ZEPHYR_INCLUDE_NET_MCP_SERVER_H_ +/** + * @file + * @brief Model Context Protocol (MCP) Server API + */ + #include #include @@ -15,6 +20,9 @@ extern "C" { #endif #ifdef CONFIG_MCP_TOOLS_CAPABILITY +/** + * @brief Tool metadata structure + */ typedef struct mcp_tool_metadata { char name[CONFIG_MCP_TOOL_NAME_MAX_LEN]; char input_schema[CONFIG_MCP_TOOL_SCHEMA_MAX_LEN]; @@ -29,10 +37,18 @@ typedef struct mcp_tool_metadata { #endif } mcp_tool_metadata_t; -/* Tool callback function signature */ +/** + * @brief Tool callback function + * + * @param params JSON string with tool parameters + * @param execution_token Unique execution identifier + * @return 0 on success, negative errno on failure + */ typedef int (*mcp_tool_callback_t)(const char *params, uint32_t execution_token); -/* Tool definition structure */ +/** + * @brief Tool definition structure + */ typedef struct mcp_tool_record { mcp_tool_metadata_t metadata; mcp_tool_callback_t callback; @@ -41,50 +57,48 @@ typedef struct mcp_tool_record { struct mcp_message_msg { uint32_t token; - /* More fields will be added later */ }; /** - * @brief Initialize the MCP Server. + * @brief Initialize the MCP Server * - * @return 0 on success, negative errno code on failure + * @return 0 on success, negative errno on failure */ int mcp_server_init(void); /** - * @brief Start the MCP Server. + * @brief Start the MCP Server * - * @return 0 on success, negative errno code on failure + * @return 0 on success, negative errno on failure */ int mcp_server_start(void); /** - * @brief Queues a response to the MCP Server library, which takes care of sending it to - * the MCP Client. + * @brief Queue a response for delivery * - * @return 0 on success, negative errno code on failure + * @return 0 on success, negative errno on failure */ int mcp_queue_response(void); #ifdef CONFIG_MCP_TOOLS_CAPABILITY /** - * @brief Add a tool to the MCP server tool registry + * @brief Add a tool to the server * - * @param tool_record Pointer to tool record containing metadata and callback - * @return 0 on success, negative errno code on failure - * -EINVAL if tool_record is NULL or invalid - * -EEXIST if tool with same name already exists - * -ENOSPC if tool registry is full + * @param tool_record Tool definition with metadata and callback + * @return 0 on success, negative errno on failure + * @retval -EINVAL Invalid tool_record + * @retval -EEXIST Tool name already exists + * @retval -ENOSPC Registry full */ int mcp_server_add_tool(const mcp_tool_record_t *tool_record); /** - * @brief Remove a tool from the MCP server tool registry + * @brief Remove a tool from the server * - * @param tool_name Name of the tool to remove - * @return 0 on success, negative errno code on failure - * -EINVAL if tool_name is NULL - * -ENOENT if tool not found + * @param tool_name Name of tool to remove + * @return 0 on success, negative errno on failure + * @retval -EINVAL Invalid tool name + * @retval -ENOENT Tool not found */ int mcp_server_remove_tool(const char *tool_name); #endif diff --git a/subsys/net/lib/mcp/mcp_common.h b/subsys/net/lib/mcp/mcp_common.h index 916129c32085e..8bf4d2ce5a7e0 100644 --- a/subsys/net/lib/mcp/mcp_common.h +++ b/subsys/net/lib/mcp/mcp_common.h @@ -29,11 +29,10 @@ typedef enum { MCP_MSG_RESPONSE_TOOLS_CALL, #endif MCP_MSG_NOTIFICATION, - /* unknown method, unknown tool, other errors */ } mcp_queue_msg_type_t; typedef enum { - MCP_SYS_DISCONNECTION, + MCP_SYS_CLIENT_SHUTDOWN, MCP_SYS_CANCEL } mcp_system_msg_type_t; @@ -56,15 +55,12 @@ typedef enum { typedef struct mcp_system_msg { mcp_system_msg_type_t type; uint32_t request_id; + uint32_t client_id; } mcp_system_msg_t; typedef struct mcp_initialize_request { uint32_t request_id; uint32_t client_id; - - /* MCP Core ignores client params and clientInfo and won't try - * to use any client capabilities, in this implementation. - */ } mcp_initialize_request_t; #ifdef CONFIG_MCP_TOOLS_CAPABILITY @@ -76,7 +72,6 @@ typedef struct mcp_tools_list_request { typedef struct mcp_tools_call_request { uint32_t request_id; uint32_t client_id; - char name[CONFIG_MCP_TOOL_NAME_MAX_LEN]; char arguments[CONFIG_MCP_TOOL_INPUT_ARGS_MAX_LEN]; } mcp_tools_call_request_t; @@ -109,26 +104,23 @@ typedef struct mcp_server_notification { mcp_notification_method_type_t method; } mcp_server_notification_t; -/* Queue message structures using void pointers */ typedef struct mcp_request_queue_msg { mcp_queue_msg_type_t type; - void *data; /* Points to allocated message data */ + void *data; } mcp_request_queue_msg_t; typedef struct mcp_response_queue_msg { mcp_queue_msg_type_t type; - void *data; /* Points to allocated message data */ + void *data; } mcp_response_queue_msg_t; #ifdef CONFIG_MCP_TOOLS_CAPABILITY -/* Tool registry structure */ typedef struct { mcp_tool_record_t tools[CONFIG_MCP_MAX_TOOLS]; struct k_mutex registry_mutex; uint8_t tool_count; } mcp_tool_registry_t; -/* Execution context for tracking active tool executions */ typedef struct { uint32_t execution_token; uint32_t request_id; @@ -141,26 +133,13 @@ typedef struct { mcp_execution_state_t execution_state; } mcp_execution_context_t; -/* Execution registry for tracking active executions */ typedef struct { mcp_execution_context_t executions[MCP_MAX_REQUESTS]; struct k_mutex execution_mutex; } mcp_execution_registry_t; #endif -/** - * @brief Allocate memory for MCP operations - * - * @param size Size in bytes to allocate - * @return Pointer to allocated memory, or NULL on failure - */ void *mcp_alloc(size_t size); - -/** - * @brief Free memory allocated by mcp_alloc() - * - * @param ptr Pointer to memory to free, or NULL (no-op if NULL) - */ void mcp_free(void *ptr); #endif /* ZEPHYR_SUBSYS_MCP_COMMON_H_ */ diff --git a/subsys/net/lib/mcp/mcp_server.c b/subsys/net/lib/mcp/mcp_server.c index fb1e1058b4cd3..0e1b7c5089775 100644 --- a/subsys/net/lib/mcp/mcp_server.c +++ b/subsys/net/lib/mcp/mcp_server.c @@ -13,25 +13,21 @@ LOG_MODULE_REGISTER(mcp_server, CONFIG_MCP_LOG_LEVEL); -/* Configuration defaults */ - #define MCP_REQUEST_WORKERS 2 #define MCP_MESSAGE_WORKERS 2 #define MCP_REQUEST_QUEUE_SIZE 10 #define MCP_MESSAGE_QUEUE_SIZE 10 #define MCP_WORKER_PRIORITY 7 -/* MCP Client Registry structures */ - -/* MCP Lifecycle states for client connections */ +/* Client lifecycle states */ typedef enum { + MCP_LIFECYCLE_DEINITIALIZED = 0, MCP_LIFECYCLE_NEW, MCP_LIFECYCLE_INITIALIZING, MCP_LIFECYCLE_INITIALIZED, - MCP_LIFECYCLE_DEINITIALIZING, + MCP_LIFECYCLE_DEINITIALIZING } mcp_lifecycle_state_t; -/* Client connection context */ typedef struct { uint32_t client_id; mcp_lifecycle_state_t lifecycle_state; @@ -39,418 +35,420 @@ typedef struct { uint8_t active_request_count; } mcp_client_context_t; -/* MCP Client Registry */ typedef struct { mcp_client_context_t clients[CONFIG_HTTP_SERVER_MAX_CLIENTS]; struct k_mutex registry_mutex; + uint8_t client_count; } mcp_client_registry_t; -/* Global client registry instance */ static mcp_client_registry_t client_registry; static mcp_tool_registry_t tool_registry; -/* Message queues */ K_MSGQ_DEFINE(mcp_request_queue, sizeof(mcp_request_queue_msg_t), MCP_REQUEST_QUEUE_SIZE, 4); K_MSGQ_DEFINE(mcp_message_queue, sizeof(mcp_response_queue_msg_t), MCP_MESSAGE_QUEUE_SIZE, 4); -/* Worker thread stacks */ K_THREAD_STACK_ARRAY_DEFINE(mcp_request_worker_stacks, MCP_REQUEST_WORKERS, 2048); K_THREAD_STACK_ARRAY_DEFINE(mcp_message_worker_stacks, MCP_MESSAGE_WORKERS, 2048); -/* Worker thread structures */ static struct k_thread mcp_request_workers[MCP_REQUEST_WORKERS]; static struct k_thread mcp_message_workers[MCP_MESSAGE_WORKERS]; -/* Request worker function */ -static void mcp_request_worker(void *arg1, void *arg2, void *arg3) +/* Must be called with registry_mutex held */ +static void cleanup_client_registry_entry(int client_index) +{ + client_registry.clients[client_index].client_id = 0; + client_registry.clients[client_index].active_request_count = 0; + + for (int i = 0; i < MCP_MAX_REQUESTS; i++) { + client_registry.clients[client_index].active_requests[i] = 0; + } + + client_registry.clients[client_index].lifecycle_state = MCP_LIFECYCLE_DEINITIALIZED; + client_registry.client_count--; +} + +static int handle_system_message(mcp_system_msg_t *system_msg) { - mcp_request_queue_msg_t request; - bool client_registered = false; - bool client_initialized = false; - int client_index = -1; - int worker_id = POINTER_TO_INT(arg1); int ret; + int client_index = -1; - LOG_INF("Request worker %d started", worker_id); + LOG_DBG("Processing system request"); - while (1) { - ret = k_msgq_get(&mcp_request_queue, &request, K_FOREVER); + switch (system_msg->type) { + case MCP_SYS_CLIENT_SHUTDOWN: + ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); if (ret != 0) { - LOG_ERR("Failed to get request from queue: %d", ret); - continue; + LOG_ERR("Failed to lock client registry mutex: %d", ret); + return ret; } - LOG_DBG("Processing request (worker %d)", worker_id); + for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { + if (client_registry.clients[i].client_id == system_msg->client_id) { + client_index = i; + client_registry.clients[i].lifecycle_state = + MCP_LIFECYCLE_DEINITIALIZING; + break; + } + } - if (request.data == NULL) { - LOG_ERR("Received NULL data pointer in request"); - continue; + if (client_index == -1) { + LOG_ERR("Client not registered"); + k_mutex_unlock(&client_registry.registry_mutex); + return -ENOENT; } - switch (request.type) { - case MCP_MSG_SYSTEM: { - mcp_system_msg_t *sys_msg; - LOG_DBG("Processing system request"); - sys_msg = (mcp_system_msg_t *)request.data; - /* Add system request handling logic */ - mcp_free(request.data); - } break; + /* TODO: Cancel active tool executions */ + cleanup_client_registry_entry(client_index); + k_mutex_unlock(&client_registry.registry_mutex); + break; - case MCP_MSG_REQUEST_INITIALIZE: { - mcp_initialize_request_t *rpc_request; - mcp_initialize_response_t *response; - mcp_response_queue_msg_t queue_response; + default: + LOG_ERR("Unknown system message type: %u", system_msg->type); + return -EINVAL; + } - LOG_DBG("Processing RPC request"); - rpc_request = (mcp_initialize_request_t *)request.data; + return 0; +} - /* Reset client tracking variables */ - client_registered = false; - client_index = -1; +static int handle_initialize_request(mcp_initialize_request_t *request) +{ + mcp_initialize_response_t *response_data; + mcp_response_queue_msg_t response; + int client_index = -1; + int ret; - /* Add RPC request handling logic */ - ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); - if (ret != 0) { - LOG_ERR("Failed to lock client registry mutex: %d", ret); - mcp_free(rpc_request); - break; - } + LOG_DBG("Processing initialize request"); - /* Search for existing client */ - for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { - if (client_registry.clients[i].client_id == - rpc_request->client_id) { - client_registered = true; - client_index = i; - LOG_DBG("Found client with ID %u", rpc_request->client_id); - break; - } - } + ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock client registry mutex: %d", ret); + return ret; + } - /* Register new client if not found */ - if (!client_registered) { - for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { - if (client_registry.clients[i].client_id == 0) { - /* Found an empty slot in the client registry */ - client_registry.clients[i].client_id = - rpc_request->client_id; - client_registry.clients[i].lifecycle_state = - MCP_LIFECYCLE_NEW; - client_registry.clients[i].active_request_count = 0; - - client_index = i; - LOG_DBG("Registered new client with ID %u", - rpc_request->client_id); - break; - } - } - - if (client_index == -1) { - LOG_ERR("Client registry is full. Cannot register new " - "client."); - k_mutex_unlock(&client_registry.registry_mutex); - mcp_free(rpc_request); - break; - } - } + /* Search for existing client */ + for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { + if (client_registry.clients[i].client_id == request->client_id) { + client_index = i; + break; + } + } - if (client_registry.clients[client_index].lifecycle_state == - MCP_LIFECYCLE_NEW) { - client_registry.clients[client_index].lifecycle_state = - MCP_LIFECYCLE_INITIALIZING; - LOG_DBG("Client %u initializing", rpc_request->client_id); - } else { - LOG_ERR("Client %u already initialized", rpc_request->client_id); - k_mutex_unlock(&client_registry.registry_mutex); - mcp_free(rpc_request); - break; - } + /* Register new client if needed */ + if (client_index == -1) { + if (client_registry.client_count >= CONFIG_HTTP_SERVER_MAX_CLIENTS) { + LOG_ERR("Client registry full"); k_mutex_unlock(&client_registry.registry_mutex); + return -ENOSPC; + } - response = (mcp_initialize_response_t *)mcp_alloc( - sizeof(mcp_initialize_response_t)); - if (!response) { - LOG_ERR("Failed to allocate response for client %u", - rpc_request->client_id); - /* TODO: Queue error response */ - mcp_free(rpc_request); + for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { + if (client_registry.clients[i].client_id == 0) { + client_registry.clients[i].client_id = request->client_id; + client_registry.clients[i].lifecycle_state = MCP_LIFECYCLE_NEW; + client_registry.clients[i].active_request_count = 0; + client_index = i; + client_registry.client_count++; break; } + } + } + + /* State transition: NEW -> INITIALIZING */ + if (client_registry.clients[client_index].lifecycle_state == MCP_LIFECYCLE_NEW) { + client_registry.clients[client_index].lifecycle_state = MCP_LIFECYCLE_INITIALIZING; + } else { + LOG_ERR("Client %u invalid state for initialization", request->client_id); + k_mutex_unlock(&client_registry.registry_mutex); + return -EALREADY; + } - response->request_id = rpc_request->request_id; + k_mutex_unlock(&client_registry.registry_mutex); - /* Set capabilities based on enabled features */ - response->capabilities = 0; + response_data = (mcp_initialize_response_t *)mcp_alloc(sizeof(mcp_initialize_response_t)); + if (!response_data) { + LOG_ERR("Failed to allocate response"); + return -ENOMEM; + } + + response_data->request_id = request->request_id; + response_data->capabilities = 0; #ifdef CONFIG_MCP_TOOLS_CAPABILITY - response->capabilities |= MCP_TOOLS; + response_data->capabilities |= MCP_TOOLS; #endif - queue_response.type = MCP_MSG_RESPONSE_INITIALIZE; - queue_response.data = response; + response.type = MCP_MSG_RESPONSE_INITIALIZE; + response.data = response_data; - ret = mcp_transport_queue_response(&queue_response); - if (ret != 0) { - LOG_ERR("Failed to queue response: %d", ret); - mcp_free(response); - } + ret = mcp_transport_queue_response(&response); + if (ret != 0) { + LOG_ERR("Failed to queue response: %d", ret); + mcp_free(response_data); + return ret; + } - mcp_free(rpc_request); - break; - } + return 0; +} #ifdef CONFIG_MCP_TOOLS_CAPABILITY - case MCP_MSG_REQUEST_TOOLS_LIST: { - mcp_tools_list_request_t *rpc_request; - mcp_tools_list_response_t *response; - mcp_response_queue_msg_t queue_response; +static int handle_tools_list_request(mcp_tools_list_request_t *request) +{ + mcp_tools_list_response_t *response_data; + mcp_response_queue_msg_t response; + int client_index = -1; + int ret; - LOG_DBG("Processing RPC request"); - rpc_request = (mcp_tools_list_request_t *)request.data; + LOG_DBG("Processing tools list request"); - /* Reset client tracking variables */ - client_registered = false; - client_initialized = false; + /* Check client state in single lock operation */ + ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock client registry mutex: %d", ret); + return ret; + } - /* Add RPC request handling logic */ - ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); - if (ret != 0) { - LOG_ERR("Failed to lock client registry mutex: %d", ret); - mcp_free(rpc_request); - break; - } + for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { + if (client_registry.clients[i].client_id == request->client_id) { + client_index = i; + break; + } + } - /* Search for existing client */ - for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { - if (client_registry.clients[i].client_id == - rpc_request->client_id) { - client_registered = true; - - if (client_registry.clients[i].lifecycle_state == - MCP_LIFECYCLE_INITIALIZED) { - LOG_DBG("Client %u initialized, processing request", - rpc_request->client_id); - - client_initialized = true; - } - break; - } - } + if (client_index == -1) { + k_mutex_unlock(&client_registry.registry_mutex); + return -ENOENT; + } - if (!client_registered) { - LOG_ERR("Client not registered. Refusing to process tools list " - "request."); - /* TODO: Handle error response */ - k_mutex_unlock(&client_registry.registry_mutex); - mcp_free(rpc_request); - break; - } + if (client_registry.clients[client_index].lifecycle_state != MCP_LIFECYCLE_INITIALIZED) { + k_mutex_unlock(&client_registry.registry_mutex); + return -EPERM; + } - if (!client_initialized) { - LOG_ERR("Client not initialized. Refusing to process tools list " - "request."); - /* TODO: Handle error response */ - k_mutex_unlock(&client_registry.registry_mutex); - mcp_free(rpc_request); - break; - } + k_mutex_unlock(&client_registry.registry_mutex); - k_mutex_unlock(&client_registry.registry_mutex); + response_data = (mcp_tools_list_response_t *)mcp_alloc(sizeof(mcp_tools_list_response_t)); + if (!response_data) { + LOG_ERR("Failed to allocate response"); + return -ENOMEM; + } - LOG_DBG("Retrieving tools list for client %u", rpc_request->client_id); - response = (mcp_tools_list_response_t *)mcp_alloc( - sizeof(mcp_tools_list_response_t)); - if (!response) { - LOG_ERR("Failed to allocate tools list response for client %u", - rpc_request->client_id); - mcp_free(rpc_request); - /* TODO: Queue error response */ - break; - } + response_data->request_id = request->request_id; - response->request_id = rpc_request->request_id; - /* Lock tool registry and copy tool information */ - ret = k_mutex_lock(&tool_registry.registry_mutex, K_FOREVER); - if (ret != 0) { - LOG_ERR("Failed to lock tool registry mutex: %d", ret); - mcp_free(rpc_request); - mcp_free(response); - break; - } + ret = k_mutex_lock(&tool_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock tool registry"); + mcp_free(response_data); + return ret; + } + + response_data->tool_count = tool_registry.tool_count; + + /* Copy tool metadata */ + for (int i = 0; i < tool_registry.tool_count; i++) { + strncpy(response_data->tools[i].name, tool_registry.tools[i].metadata.name, + CONFIG_MCP_TOOL_NAME_MAX_LEN - 1); + response_data->tools[i].name[CONFIG_MCP_TOOL_NAME_MAX_LEN - 1] = '\0'; - response->tool_count = tool_registry.tool_count; - /* Copy tool metadata for transport layer */ - for (int i = 0; i < tool_registry.tool_count; i++) { - /* Required fields - always copy */ - strncpy(response->tools[i].name, - tool_registry.tools[i].metadata.name, - CONFIG_MCP_TOOL_NAME_MAX_LEN - 1); - response->tools[i].name[CONFIG_MCP_TOOL_NAME_MAX_LEN - 1] = '\0'; - - strncpy(response->tools[i].input_schema, - tool_registry.tools[i].metadata.input_schema, - CONFIG_MCP_TOOL_SCHEMA_MAX_LEN - 1); - response->tools[i] - .input_schema[CONFIG_MCP_TOOL_SCHEMA_MAX_LEN - 1] = '\0'; + strncpy(response_data->tools[i].input_schema, + tool_registry.tools[i].metadata.input_schema, + CONFIG_MCP_TOOL_SCHEMA_MAX_LEN - 1); + response_data->tools[i].input_schema[CONFIG_MCP_TOOL_SCHEMA_MAX_LEN - 1] = '\0'; #ifdef CONFIG_MCP_TOOL_DESC - /* Description - only copy if not empty */ - if (strlen(tool_registry.tools[i].metadata.description) > 0) { - strncpy(response->tools[i].description, - tool_registry.tools[i].metadata.description, - CONFIG_MCP_TOOL_DESC_MAX_LEN - 1); - response->tools[i] - .description[CONFIG_MCP_TOOL_DESC_MAX_LEN - 1] = - '\0'; - } else { - response->tools[i].description[0] = '\0'; - } + if (strlen(tool_registry.tools[i].metadata.description) > 0) { + strncpy(response_data->tools[i].description, + tool_registry.tools[i].metadata.description, + CONFIG_MCP_TOOL_DESC_MAX_LEN - 1); + response_data->tools[i].description[CONFIG_MCP_TOOL_DESC_MAX_LEN - 1] = + '\0'; + } else { + response_data->tools[i].description[0] = '\0'; + } #endif #ifdef CONFIG_MCP_TOOL_TITLE - /* Title - only copy if not empty */ - if (strlen(tool_registry.tools[i].metadata.title) > 0) { - strncpy(response->tools[i].title, - tool_registry.tools[i].metadata.title, - CONFIG_MCP_TOOL_NAME_MAX_LEN - 1); - response->tools[i].title[CONFIG_MCP_TOOL_NAME_MAX_LEN - 1] = - '\0'; - } else { - response->tools[i].title[0] = '\0'; - } + if (strlen(tool_registry.tools[i].metadata.title) > 0) { + strncpy(response_data->tools[i].title, + tool_registry.tools[i].metadata.title, + CONFIG_MCP_TOOL_NAME_MAX_LEN - 1); + response_data->tools[i].title[CONFIG_MCP_TOOL_NAME_MAX_LEN - 1] = '\0'; + } else { + response_data->tools[i].title[0] = '\0'; + } #endif #ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA - /* Output schema - only copy if not empty */ - if (strlen(tool_registry.tools[i].metadata.output_schema) > 0) { - strncpy(response->tools[i].output_schema, - tool_registry.tools[i].metadata.output_schema, - CONFIG_MCP_TOOL_SCHEMA_MAX_LEN - 1); - response->tools[i] - .output_schema[CONFIG_MCP_TOOL_SCHEMA_MAX_LEN - 1] = - '\0'; - } else { - response->tools[i].output_schema[0] = '\0'; - } + if (strlen(tool_registry.tools[i].metadata.output_schema) > 0) { + strncpy(response_data->tools[i].output_schema, + tool_registry.tools[i].metadata.output_schema, + CONFIG_MCP_TOOL_SCHEMA_MAX_LEN - 1); + response_data->tools[i].output_schema[CONFIG_MCP_TOOL_SCHEMA_MAX_LEN - 1] = + '\0'; + } else { + response_data->tools[i].output_schema[0] = '\0'; + } #endif - } + } - k_mutex_unlock(&tool_registry.registry_mutex); + k_mutex_unlock(&tool_registry.registry_mutex); - queue_response.type = MCP_MSG_RESPONSE_TOOLS_LIST; - queue_response.data = response; + response.type = MCP_MSG_RESPONSE_TOOLS_LIST; + response.data = response_data; - ret = mcp_transport_queue_response(&queue_response); - if (ret != 0) { - LOG_ERR("Failed to queue tools list response: %d", ret); - mcp_free(rpc_request); - mcp_free(response); - break; - } + ret = mcp_transport_queue_response(&response); + if (ret != 0) { + LOG_ERR("Failed to queue response: %d", ret); + mcp_free(response_data); + return ret; + } + + return 0; +} + +static int handle_tools_call_request(mcp_tools_call_request_t *request) +{ + LOG_DBG("Tool call request for client %u", request->client_id); + /* TODO: Implement tool execution */ + return 0; +} +#endif + +static int handle_notification(mcp_client_notification_t *notification) +{ + int ret; + int client_index = -1; + + LOG_DBG("Processing notification"); + + ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock client registry"); + return ret; + } - mcp_free(rpc_request); + for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { + if (client_registry.clients[i].client_id == notification->client_id) { + client_index = i; break; } + } - case MCP_MSG_REQUEST_TOOLS_CALL: { - mcp_tools_call_request_t *rpc_request; - mcp_tools_call_response_t *response; - mcp_response_queue_msg_t queue_response; + if (client_index == -1) { + LOG_ERR("Client not found"); + k_mutex_unlock(&client_registry.registry_mutex); + return -ENOENT; + } - rpc_request = (mcp_tools_call_request_t *)request.data; - LOG_DBG("Calling tool for client %u", rpc_request->client_id); - /* TODO: Implement tool call handling */ + switch (notification->method) { + case MCP_NOTIF_INITIALIZED: + /* State transition: INITIALIZING -> INITIALIZED */ + if (client_registry.clients[client_index].lifecycle_state == + MCP_LIFECYCLE_INITIALIZING) { + client_registry.clients[client_index].lifecycle_state = + MCP_LIFECYCLE_INITIALIZED; + } else { + LOG_ERR("Invalid state transition for client %u", notification->client_id); + k_mutex_unlock(&client_registry.registry_mutex); + return -EPERM; + } + break; - mcp_free(rpc_request); - break; + default: + LOG_ERR("Unknown notification method %u", notification->method); + k_mutex_unlock(&client_registry.registry_mutex); + return -EINVAL; + } + + k_mutex_unlock(&client_registry.registry_mutex); + return 0; +} + +static void mcp_request_worker(void *arg1, void *arg2, void *arg3) +{ + mcp_request_queue_msg_t request; + int worker_id = POINTER_TO_INT(arg1); + int ret; + + LOG_INF("Request worker %d started", worker_id); + + while (1) { + ret = k_msgq_get(&mcp_request_queue, &request, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to get request: %d", ret); + continue; } -#endif - case MCP_MSG_NOTIFICATION: { - mcp_client_notification_t *notification; - LOG_DBG("Processing RPC notification"); - notification = (mcp_client_notification_t *)request.data; + if (request.data == NULL) { + LOG_ERR("NULL data in request"); + continue; + } - /* Reset client tracking variables */ - client_registered = false; - client_index = -1; + switch (request.type) { + case MCP_MSG_SYSTEM: { + mcp_system_msg_t *system_msg = (mcp_system_msg_t *)request.data; - /* Add RPC notification handling logic */ - ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); + ret = handle_system_message(system_msg); if (ret != 0) { - LOG_ERR("Failed to lock client registry mutex: %d", ret); - mcp_free(notification); - break; + LOG_ERR("System message failed: %d", ret); } + mcp_free(system_msg); + break; + } - /* Search for client */ - for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { - if (client_registry.clients[i].client_id == - notification->client_id) { - client_registered = true; - client_index = i; - LOG_DBG("Found client with ID %u", notification->client_id); - break; - } - } + case MCP_MSG_REQUEST_INITIALIZE: { + mcp_initialize_request_t *req = (mcp_initialize_request_t *)request.data; - if (!client_registered) { - LOG_ERR("Client not registered. Refusing to process notification."); - /* TODO: Handle error response */ - k_mutex_unlock(&client_registry.registry_mutex); - mcp_free(notification); - break; + ret = handle_initialize_request(req); + if (ret != 0) { + LOG_ERR("Initialize request failed: %d", ret); } + mcp_free(req); + break; + } - k_mutex_unlock(&client_registry.registry_mutex); +#ifdef CONFIG_MCP_TOOLS_CAPABILITY + case MCP_MSG_REQUEST_TOOLS_LIST: { + mcp_tools_list_request_t *req = (mcp_tools_list_request_t *)request.data; - switch (notification->method) { - case MCP_NOTIF_INITIALIZED: { - ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); - if (ret != 0) { - LOG_ERR("Failed to lock client registry mutex: %d", ret); - break; - } - - if (client_registry.clients[client_index].lifecycle_state == - MCP_LIFECYCLE_INITIALIZING) { - client_registry.clients[client_index].lifecycle_state = - MCP_LIFECYCLE_INITIALIZED; - LOG_DBG("Client %u initialization complete", - notification->client_id); - } else { - LOG_ERR("Invalid state transition for client %u", - notification->client_id); - /* TODO: Respond with an error message */ - k_mutex_unlock(&client_registry.registry_mutex); - break; - } - k_mutex_unlock(&client_registry.registry_mutex); - break; + ret = handle_tools_list_request(req); + if (ret != 0) { + LOG_ERR("Tools list request failed: %d", ret); } + mcp_free(req); + break; + } - default: { - LOG_ERR("Unknown notification method %u", notification->method); - /* TODO: Respond with an error message */ - break; - } - } + case MCP_MSG_REQUEST_TOOLS_CALL: { + mcp_tools_call_request_t *req = (mcp_tools_call_request_t *)request.data; - mcp_free(notification); + ret = handle_tools_call_request(req); + if (ret != 0) { + LOG_ERR("Tools call request failed: %d", ret); + } + mcp_free(req); break; } +#endif - default: { - LOG_ERR("Unknown message type %u", request.type); - if (request.data) { - mcp_free(request.data); + case MCP_MSG_NOTIFICATION: { + mcp_client_notification_t *notif = + (mcp_client_notification_t *)request.data; + + ret = handle_notification(notif); + if (ret != 0) { + LOG_ERR("Notification failed: %d", ret); } + mcp_free(notif); break; } + + default: + LOG_ERR("Unknown message type %u", request.type); + mcp_free(request.data); + break; } } } -/* Message worker function */ static void mcp_message_worker(void *arg1, void *arg2, void *arg3) { struct mcp_message_msg msg; @@ -465,7 +463,7 @@ static void mcp_message_worker(void *arg1, void *arg2, void *arg3) LOG_DBG("Processing message (worker %d)", worker_id); /* TODO: Implement message processing */ } else { - LOG_ERR("Failed to get message from queue: %d", ret); + LOG_ERR("Failed to get message: %d", ret); } } } @@ -474,31 +472,25 @@ int mcp_server_init(void) { int ret; - LOG_INF("Initializing MCP Server Core"); + LOG_INF("Initializing MCP Server"); - /* Initialize client registry mutex */ ret = k_mutex_init(&client_registry.registry_mutex); if (ret != 0) { - LOG_ERR("Failed to initialize client registry mutex: %d", ret); + LOG_ERR("Failed to init client mutex: %d", ret); return ret; } - /* Initialize tool registry mutex */ ret = k_mutex_init(&tool_registry.registry_mutex); if (ret != 0) { - LOG_ERR("Failed to initialize tool registry mutex: %d", ret); + LOG_ERR("Failed to init tool mutex: %d", ret); return ret; } - /* Initialize client registry */ memset(&client_registry.clients, 0, sizeof(client_registry.clients)); - - /* Initialize tool registry */ tool_registry.tool_count = 0; memset(&tool_registry.tools, 0, sizeof(tool_registry.tools)); - LOG_INF("MCP Server Core initialized"); - + LOG_INF("MCP Server initialized"); return 0; } @@ -507,45 +499,49 @@ int mcp_server_start(void) k_tid_t tid; int ret; - LOG_INF("Starting MCP Server Core"); + LOG_INF("Starting MCP Server"); - /* Create and start request worker threads */ for (int i = 0; i < MCP_REQUEST_WORKERS; i++) { tid = k_thread_create(&mcp_request_workers[i], mcp_request_worker_stacks[i], K_THREAD_STACK_SIZEOF(mcp_request_worker_stacks[i]), mcp_request_worker, INT_TO_POINTER(i), NULL, NULL, K_PRIO_COOP(MCP_WORKER_PRIORITY), 0, K_NO_WAIT); if (tid == NULL) { - LOG_ERR("Failed to create request worker thread %d", i); + LOG_ERR("Failed to create request worker %d", i); return -ENOMEM; } ret = k_thread_name_set(&mcp_request_workers[i], "mcp_req_worker"); if (ret != 0) { - LOG_WRN("Failed to set name for request worker thread %d: %d", i, ret); + LOG_WRN("Failed to set thread name: %d", ret); } } - /* Create and start message worker threads */ for (int i = 0; i < MCP_MESSAGE_WORKERS; i++) { tid = k_thread_create(&mcp_message_workers[i], mcp_message_worker_stacks[i], K_THREAD_STACK_SIZEOF(mcp_message_worker_stacks[i]), mcp_message_worker, INT_TO_POINTER(i), NULL, NULL, K_PRIO_COOP(MCP_WORKER_PRIORITY), 0, K_NO_WAIT); if (tid == NULL) { - LOG_ERR("Failed to create message worker thread %d", i); + LOG_ERR("Failed to create message worker %d", i); return -ENOMEM; } ret = k_thread_name_set(&mcp_message_workers[i], "mcp_msg_worker"); if (ret != 0) { - LOG_WRN("Failed to set name for message worker thread %d: %d", i, ret); + LOG_WRN("Failed to set thread name: %d", ret); } } - LOG_INF("MCP Server Core started: %d request workers, %d message workers", - MCP_REQUEST_WORKERS, MCP_MESSAGE_WORKERS); + LOG_INF("MCP Server started: %d request, %d message workers", MCP_REQUEST_WORKERS, + MCP_MESSAGE_WORKERS); + + return 0; +} +int mcp_queue_response(void) +{ + /* TODO: Implement response queuing */ return 0; } @@ -556,89 +552,82 @@ int mcp_server_add_tool(const mcp_tool_record_t *tool_record) int available_slot = -1; if (!tool_record || !tool_record->metadata.name[0] || !tool_record->callback) { - LOG_ERR("Invalid tool info provided"); + LOG_ERR("Invalid tool record"); return -EINVAL; } ret = k_mutex_lock(&tool_registry.registry_mutex, K_FOREVER); if (ret != 0) { - LOG_ERR("Failed to lock tool registry mutex: %d", ret); + LOG_ERR("Failed to lock tool registry: %d", ret); return ret; } - /* Check for duplicate names and find first available slot */ + /* Find available slot and check for duplicates */ for (int i = 0; i < CONFIG_MCP_MAX_TOOLS; i++) { - /* Check if slot is empty (available) */ if (tool_registry.tools[i].metadata.name[0] == '\0' && available_slot == -1) { available_slot = i; } - /* Check for duplicate tool name */ if (tool_registry.tools[i].metadata.name[0] != '\0' && strncmp(tool_registry.tools[i].metadata.name, tool_record->metadata.name, CONFIG_MCP_TOOL_NAME_MAX_LEN) == 0) { - LOG_ERR("Tool with name '%s' already exists", tool_record->metadata.name); + LOG_ERR("Tool '%s' already exists", tool_record->metadata.name); k_mutex_unlock(&tool_registry.registry_mutex); return -EEXIST; } } - /* Check if registry is full */ if (available_slot == -1) { - LOG_ERR("Tool registry is full. Cannot add tool '%s'", tool_record->metadata.name); + LOG_ERR("Tool registry full"); k_mutex_unlock(&tool_registry.registry_mutex); return -ENOSPC; } - /* Copy the tool record to the registry */ memcpy(&tool_registry.tools[available_slot], tool_record, sizeof(mcp_tool_record_t)); tool_registry.tool_count++; - LOG_INF("Tool '%s' registered successfully at slot %d", tool_record->metadata.name, - available_slot); + LOG_INF("Tool '%s' registered at slot %d", tool_record->metadata.name, available_slot); k_mutex_unlock(&tool_registry.registry_mutex); - return 0; } int mcp_server_remove_tool(const char *tool_name) { int ret; - bool found = false; + int tool_index = -1; if (!tool_name || !tool_name[0]) { - LOG_ERR("Invalid tool name provided"); + LOG_ERR("Invalid tool name"); return -EINVAL; } ret = k_mutex_lock(&tool_registry.registry_mutex, K_FOREVER); if (ret != 0) { - LOG_ERR("Failed to lock tool registry mutex: %d", ret); + LOG_ERR("Failed to lock tool registry: %d", ret); return ret; } - /* Find and remove the tool */ for (int i = 0; i < CONFIG_MCP_MAX_TOOLS; i++) { if (tool_registry.tools[i].metadata.name[0] != '\0' && strncmp(tool_registry.tools[i].metadata.name, tool_name, CONFIG_MCP_TOOL_NAME_MAX_LEN) == 0) { - /* Clear the tool slot */ - memset(&tool_registry.tools[i], 0, sizeof(mcp_tool_record_t)); - tool_registry.tool_count--; - found = true; - LOG_INF("Tool '%s' removed successfully", tool_name); + tool_index = i; break; } } - k_mutex_unlock(&tool_registry.registry_mutex); - - if (!found) { + if (tool_index == -1) { + k_mutex_unlock(&tool_registry.registry_mutex); LOG_ERR("Tool '%s' not found", tool_name); return -ENOENT; } + memset(&tool_registry.tools[tool_index], 0, sizeof(mcp_tool_record_t)); + tool_registry.tool_count--; + LOG_INF("Tool '%s' removed", tool_name); + + k_mutex_unlock(&tool_registry.registry_mutex); return 0; } #endif From a7587e4191a851713a4a9d2218deeaa1b14bf6ae Mon Sep 17 00:00:00 2001 From: David Piskula Date: Tue, 4 Nov 2025 15:05:29 +0100 Subject: [PATCH 06/12] networking: unit tests Added unit tests for implemented features Signed-off-by: David Piskula --- subsys/net/lib/mcp/mcp_server.c | 12 + subsys/net/lib/mcp/mcp_transport.c | 14 + tests/net/lib/mcp/CMakeLists.txt | 11 + tests/net/lib/mcp/prj.conf | 27 ++ tests/net/lib/mcp/src/main.c | 699 +++++++++++++++++++++++++++++ tests/net/lib/mcp/testcase.yaml | 5 + 6 files changed, 768 insertions(+) create mode 100644 tests/net/lib/mcp/CMakeLists.txt create mode 100644 tests/net/lib/mcp/prj.conf create mode 100644 tests/net/lib/mcp/src/main.c create mode 100644 tests/net/lib/mcp/testcase.yaml diff --git a/subsys/net/lib/mcp/mcp_server.c b/subsys/net/lib/mcp/mcp_server.c index 0e1b7c5089775..970aa89d13dfd 100644 --- a/subsys/net/lib/mcp/mcp_server.c +++ b/subsys/net/lib/mcp/mcp_server.c @@ -631,3 +631,15 @@ int mcp_server_remove_tool(const char *tool_name) return 0; } #endif + +#ifdef CONFIG_ZTEST +uint8_t mcp_server_get_client_count(void) +{ + return client_registry.client_count; +} + +uint8_t mcp_server_get_tool_count(void) +{ + return tool_registry.tool_count; +} +#endif diff --git a/subsys/net/lib/mcp/mcp_transport.c b/subsys/net/lib/mcp/mcp_transport.c index b0b132a431b04..f8721790a0dd8 100644 --- a/subsys/net/lib/mcp/mcp_transport.c +++ b/subsys/net/lib/mcp/mcp_transport.c @@ -9,8 +9,22 @@ #include #include "mcp_transport.h" +#ifdef CONFIG_ZTEST +int mcp_transport_queue_call_count; +mcp_response_queue_msg_t mcp_transport_last_queued_msg = {0}; +#endif + int mcp_transport_queue_response(mcp_response_queue_msg_t *msg) { +#ifdef CONFIG_ZTEST + /* Store call information for testing */ + mcp_transport_queue_call_count++; + if (msg) { + mcp_transport_last_queued_msg.type = msg->type; + mcp_transport_last_queued_msg.data = msg->data; + } +#endif + /* queue msg to the correct requests queue */ return 0; } diff --git a/tests/net/lib/mcp/CMakeLists.txt b/tests/net/lib/mcp/CMakeLists.txt new file mode 100644 index 0000000000000..15af115fe580c --- /dev/null +++ b/tests/net/lib/mcp/CMakeLists.txt @@ -0,0 +1,11 @@ +# SPDX-License-Identifier: Apache-2.0 + +cmake_minimum_required(VERSION 3.20.0) +find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) +project(mcp_tests) +target_sources(app PRIVATE src/main.c) + +target_include_directories(app PRIVATE + ${ZEPHYR_BASE}/subsys/net/lib/mcp + ${ZEPHYR_BASE}/include +) diff --git a/tests/net/lib/mcp/prj.conf b/tests/net/lib/mcp/prj.conf new file mode 100644 index 0000000000000..6ef2a4a042a73 --- /dev/null +++ b/tests/net/lib/mcp/prj.conf @@ -0,0 +1,27 @@ +CONFIG_ZTEST=y +CONFIG_MCP_SERVER=y +CONFIG_MCP_TOOLS_CAPABILITY=y +CONFIG_MCP_MAX_TOOLS=4 +CONFIG_MCP_TOOL_DESC=y +CONFIG_MCP_SERVER_INFO_TITLE=y +CONFIG_MCP_SERVER_INFO_INSTRUCTIONS=y +CONFIG_MCP_TOOL_DESC=y +CONFIG_MCP_TOOL_NAME_MAX_LEN=32 +CONFIG_MCP_TOOL_SCHEMA_MAX_LEN=512 +CONFIG_MCP_TOOL_INPUT_ARGS_MAX_LEN=512 +CONFIG_MCP_TOOL_RESULT_MAX_LEN=256 +CONFIG_HTTP_SERVER_MAX_CLIENTS=3 +CONFIG_HTTP_SERVER_MAX_STREAMS=3 +CONFIG_MCP_LOG_LEVEL_DBG=y +CONFIG_NETWORKING=y +CONFIG_HTTP_SERVER=y +CONFIG_JSON_LIBRARY=y + +# Enable kernel heap for k_malloc/k_free +CONFIG_HEAP_MEM_POOL_SIZE=16384 +CONFIG_KERNEL_MEM_POOL=y + +# Increase stack sizes to prevent stack overflow +CONFIG_MAIN_STACK_SIZE=4096 +CONFIG_SYSTEM_WORKQUEUE_STACK_SIZE=4096 +CONFIG_TEST_EXTRA_STACK_SIZE=2048 diff --git a/tests/net/lib/mcp/src/main.c b/tests/net/lib/mcp/src/main.c new file mode 100644 index 0000000000000..e853216143e8f --- /dev/null +++ b/tests/net/lib/mcp/src/main.c @@ -0,0 +1,699 @@ +/* + * Copyright 2025 NXP + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include +#include + +#include "mcp_common.h" +#include "mcp_transport.h" + +extern struct k_msgq mcp_request_queue; +extern struct k_msgq mcp_message_queue; +extern int mcp_transport_queue_call_count; +extern mcp_response_queue_msg_t mcp_transport_last_queued_msg; + +#ifdef CONFIG_ZTEST +extern uint8_t mcp_server_get_client_count(void); +extern uint8_t mcp_server_get_tool_count(void); +#endif + +#define CLIENT_ID_BASE 1000 +#define CLIENT_ID_LIFECYCLE_TEST (CLIENT_ID_BASE + 1) +#define CLIENT_ID_INITIALIZE_TEST (CLIENT_ID_BASE + 2) +#define CLIENT_ID_EDGE_CASE_TEST (CLIENT_ID_BASE + 3) +#define CLIENT_ID_SHUTDOWN_TEST (CLIENT_ID_BASE + 4) +#define CLIENT_ID_INVALID_STATE_TEST (CLIENT_ID_BASE + 5) +#define CLIENT_ID_MULTI_CLIENT_1 (CLIENT_ID_BASE + 6) +#define CLIENT_ID_MULTI_CLIENT_2 (CLIENT_ID_BASE + 7) +#define CLIENT_ID_MULTI_CLIENT_3 (CLIENT_ID_BASE + 8) +#define CLIENT_ID_MULTI_CLIENT_4 (CLIENT_ID_BASE + 9) +#define CLIENT_ID_UNREGISTERED (CLIENT_ID_BASE + 999) + +#define REQUEST_ID_BASE 2000 + +#define REQ_ID_EDGE_CASE_UNREGISTERED (REQUEST_ID_BASE + 1) +#define REQ_ID_EDGE_CASE_INITIALIZE (REQUEST_ID_BASE + 2) +#define REQ_ID_EDGE_CASE_TOOLS_LIST (REQUEST_ID_BASE + 3) + +#define REQ_ID_INITIALIZE_TEST (REQUEST_ID_BASE + 10) + +#define REQ_ID_LIFECYCLE_INITIALIZE (REQUEST_ID_BASE + 20) +#define REQ_ID_LIFECYCLE_TOOLS_INIT (REQUEST_ID_BASE + 21) +#define REQ_ID_LIFECYCLE_TOOLS_READY (REQUEST_ID_BASE + 22) + +#define REQ_ID_SHUTDOWN_INITIALIZE (REQUEST_ID_BASE + 30) +#define REQ_ID_SHUTDOWN_TOOLS_ACTIVE (REQUEST_ID_BASE + 31) +#define REQ_ID_SHUTDOWN_TOOLS_DEAD (REQUEST_ID_BASE + 32) + +#define REQ_ID_INVALID_INITIALIZE (REQUEST_ID_BASE + 40) +#define REQ_ID_INVALID_REINITIALIZE (REQUEST_ID_BASE + 41) + +#define REQ_ID_MULTI_CLIENT_1_INIT (REQUEST_ID_BASE + 50) +#define REQ_ID_MULTI_CLIENT_2_INIT (REQUEST_ID_BASE + 51) +#define REQ_ID_MULTI_CLIENT_3_INIT (REQUEST_ID_BASE + 52) +#define REQ_ID_MULTI_CLIENT_4_INIT_1 (REQUEST_ID_BASE + 53) +#define REQ_ID_MULTI_CLIENT_4_INIT_2 (REQUEST_ID_BASE + 54) + +#define MCP_MSG_INVALID_TYPE 0xFF + +static int stub_tool_callback_1(const char *params, uint32_t execution_token) +{ + return 0; +} + +static int stub_tool_callback_2(const char *params, uint32_t execution_token) +{ + return 0; +} + +static int stub_tool_callback_3(const char *params, uint32_t execution_token) +{ + return 0; +} + +static void reset_transport_mock(void) +{ + mcp_transport_queue_call_count = 0; + memset(&mcp_transport_last_queued_msg, 0, sizeof(mcp_transport_last_queued_msg)); +} + +static void send_initialize_request(uint32_t client_id, uint32_t request_id) +{ + int ret; + mcp_request_queue_msg_t msg; + mcp_initialize_request_t *init_req; + + init_req = (mcp_initialize_request_t *)mcp_alloc(sizeof(mcp_initialize_request_t)); + zassert_not_null(init_req, "Initialize request allocation failed"); + + init_req->request_id = request_id; + init_req->client_id = client_id; + + msg.type = MCP_MSG_REQUEST_INITIALIZE; + msg.data = init_req; + + ret = k_msgq_put(&mcp_request_queue, &msg, K_NO_WAIT); + zassert_equal(ret, 0, "Initialize request queueing failed"); + + k_msleep(50); +} + +static void send_client_shutdown(uint32_t client_id) +{ + int ret; + mcp_request_queue_msg_t msg; + mcp_system_msg_t *sys_msg; + + sys_msg = (mcp_system_msg_t *)mcp_alloc(sizeof(mcp_system_msg_t)); + zassert_not_null(sys_msg, "System message allocation failed"); + + sys_msg->type = MCP_SYS_CLIENT_SHUTDOWN; + sys_msg->client_id = client_id; + + msg.type = MCP_MSG_SYSTEM; + msg.data = sys_msg; + + ret = k_msgq_put(&mcp_request_queue, &msg, K_NO_WAIT); + zassert_equal(ret, 0, "Shutdown message queueing failed"); + + k_msleep(50); +} + +static void send_initialized_notification(uint32_t client_id) +{ + int ret; + mcp_request_queue_msg_t msg; + mcp_client_notification_t *notification; + + notification = (mcp_client_notification_t *)mcp_alloc(sizeof(mcp_client_notification_t)); + zassert_not_null(notification, "Notification allocation failed"); + + notification->client_id = client_id; + notification->method = MCP_NOTIF_INITIALIZED; + + msg.type = MCP_MSG_NOTIFICATION; + msg.data = notification; + + ret = k_msgq_put(&mcp_request_queue, &msg, K_NO_WAIT); + zassert_equal(ret, 0, "Notification queueing failed"); + + k_msleep(50); +} + +static void send_tools_list_request(uint32_t client_id, uint32_t request_id) +{ + int ret; + mcp_request_queue_msg_t msg; + mcp_tools_list_request_t *tools_req; + + tools_req = (mcp_tools_list_request_t *)mcp_alloc(sizeof(mcp_tools_list_request_t)); + zassert_not_null(tools_req, "Tools request allocation failed"); + + tools_req->request_id = request_id; + tools_req->client_id = client_id; + + msg.type = MCP_MSG_REQUEST_TOOLS_LIST; + msg.data = tools_req; + + ret = k_msgq_put(&mcp_request_queue, &msg, K_NO_WAIT); + zassert_equal(ret, 0, "Tools request queueing failed"); + + k_msleep(50); +} + +static void initialize_client_fully(uint32_t client_id, uint32_t request_id) +{ + send_initialize_request(client_id, request_id); + send_initialized_notification(client_id); +} + +ZTEST(mcp_server_tests, test_memory_allocation) +{ + void *ptr1 = mcp_alloc(100); + + zassert_not_null(ptr1, "First allocation should succeed"); + + void *ptr2 = mcp_alloc(200); + + zassert_not_null(ptr2, "Second allocation should succeed"); + + zassert_not_equal(ptr1, ptr2, "Allocations should return different pointers"); + + mcp_free(ptr1); + mcp_free(ptr2); + mcp_free(NULL); +} +ZTEST(mcp_server_tests, test_tools_list_response) +{ + int ret; + uint8_t initial_tool_count = mcp_server_get_tool_count(); + + reset_transport_mock(); + + send_tools_list_request(CLIENT_ID_UNREGISTERED, REQ_ID_EDGE_CASE_UNREGISTERED); + zassert_equal(mcp_transport_queue_call_count, 0, + "Tools/list should be rejected for unregistered client"); + + mcp_tool_record_t test_tool1 = { + .metadata = { + .name = "test_tool_1", + .input_schema = "{\"type\":\"object\",\"properties\":{}}", +#ifdef CONFIG_MCP_TOOL_DESC + .description = "First test tool for verification", +#endif +#ifdef CONFIG_MCP_TOOL_TITLE + .title = "Test Tool One", +#endif +#ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA + .output_schema = "{\"type\":\"string\"}", +#endif + }, + .callback = stub_tool_callback_1}; + + mcp_tool_record_t test_tool2 = {.metadata = { + .name = "test_tool_2", + .input_schema = "{\"type\":\"array\"}", +#ifdef CONFIG_MCP_TOOL_DESC + .description = "Second test tool", +#endif +#ifdef CONFIG_MCP_TOOL_TITLE + .title = "Test Tool Two", +#endif +#ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA + .output_schema = "{\"type\":\"number\"}", +#endif + }, + .callback = stub_tool_callback_2}; + + ret = mcp_server_add_tool(&test_tool1); + zassert_equal(ret, 0, "Test tool 1 should register successfully"); + ret = mcp_server_add_tool(&test_tool2); + zassert_equal(ret, 0, "Test tool 2 should register successfully"); + + initialize_client_fully(CLIENT_ID_EDGE_CASE_TEST, REQ_ID_EDGE_CASE_INITIALIZE); + + reset_transport_mock(); + send_tools_list_request(CLIENT_ID_EDGE_CASE_TEST, REQ_ID_EDGE_CASE_TOOLS_LIST); + + zassert_equal(mcp_transport_queue_call_count, 1, "Tools/list should succeed"); + zassert_equal(mcp_transport_last_queued_msg.type, MCP_MSG_RESPONSE_TOOLS_LIST, + "Response should be tools/list type"); + + mcp_tools_list_response_t *response = + (mcp_tools_list_response_t *)mcp_transport_last_queued_msg.data; + + zassert_not_null(response, "Response data should not be NULL"); + + uint8_t expected_tool_count = initial_tool_count + 2; + + zassert_equal(response->tool_count, expected_tool_count, + "Response tool count should match registry"); + + bool found_tool1 = false; + bool found_tool2 = false; + + for (int i = 0; i < response->tool_count; i++) { + if (strcmp(response->tools[i].name, "test_tool_1") == 0) { + found_tool1 = true; + zassert_str_equal(response->tools[i].input_schema, + "{\"type\":\"object\",\"properties\":{}}", + "Tool 1 input schema should match"); +#ifdef CONFIG_MCP_TOOL_DESC + zassert_str_equal(response->tools[i].description, + "First test tool for verification", + "Tool 1 description should match"); +#endif +#ifdef CONFIG_MCP_TOOL_TITLE + zassert_str_equal(response->tools[i].title, "Test Tool One", + "Tool 1 title should match"); +#endif +#ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA + zassert_str_equal(response->tools[i].output_schema, "{\"type\":\"string\"}", + "Tool 1 output schema should match"); +#endif + } else if (strcmp(response->tools[i].name, "test_tool_2") == 0) { + found_tool2 = true; + zassert_str_equal(response->tools[i].input_schema, "{\"type\":\"array\"}", + "Tool 2 input schema should match"); +#ifdef CONFIG_MCP_TOOL_DESC + zassert_str_equal(response->tools[i].description, "Second test tool", + "Tool 2 description should match"); +#endif +#ifdef CONFIG_MCP_TOOL_TITLE + zassert_str_equal(response->tools[i].title, "Test Tool Two", + "Tool 2 title should match"); +#endif +#ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA + zassert_str_equal(response->tools[i].output_schema, "{\"type\":\"number\"}", + "Tool 2 output schema should match"); +#endif + } + + zassert_true(strlen(response->tools[i].name) > 0, "Tool name should not be empty"); + zassert_true(strlen(response->tools[i].input_schema) > 0, + "Tool input schema should not be empty"); + } + + zassert_true(found_tool1, "Test tool 1 should be found in response"); + zassert_true(found_tool2, "Test tool 2 should be found in response"); + + printk("Tool registry contains %d tools, verified tool content\n", response->tool_count); + + mcp_server_remove_tool("test_tool_1"); + mcp_server_remove_tool("test_tool_2"); + + send_client_shutdown(CLIENT_ID_EDGE_CASE_TEST); +} + +ZTEST(mcp_server_tests, test_initialize_request) +{ + reset_transport_mock(); + + send_initialize_request(CLIENT_ID_INITIALIZE_TEST, REQ_ID_INITIALIZE_TEST); + + zassert_equal(mcp_transport_queue_call_count, 1, "Transport should be called once"); + zassert_equal(mcp_transport_last_queued_msg.type, MCP_MSG_RESPONSE_INITIALIZE, + "Response type should be initialize"); + + mcp_initialize_response_t *response = + (mcp_initialize_response_t *)mcp_transport_last_queued_msg.data; + + zassert_not_null(response, "Response data should not be NULL"); + zassert_equal(response->request_id, REQ_ID_INITIALIZE_TEST, + "Response request ID should match"); + +#ifdef CONFIG_MCP_TOOLS_CAPABILITY + zassert_true(response->capabilities & MCP_TOOLS, + "Tools capability should be set when CONFIG_MCP_TOOLS_CAPABILITY is enabled"); +#endif + +#ifdef CONFIG_MCP_SERVER_INFO_TITLE + printk("Server info title feature is enabled\n"); +#endif + +#ifdef CONFIG_MCP_SERVER_INFO_INSTRUCTIONS + printk("Server info instructions feature is enabled\n"); +#endif + + send_client_shutdown(CLIENT_ID_INITIALIZE_TEST); +} +ZTEST(mcp_server_tests, test_tool_registration_valid) +{ + int ret; + uint8_t initial_count = mcp_server_get_tool_count(); + + mcp_tool_record_t valid_tool = {.metadata = { + .name = "test_tool_valid", + .input_schema = "{\"type\":\"object\"}", +#ifdef CONFIG_MCP_TOOL_DESC + .description = "Test tool description", +#endif +#ifdef CONFIG_MCP_TOOL_TITLE + .title = "Test Tool", +#endif +#ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA + .output_schema = "{\"type\":\"object\"}", +#endif + }, + .callback = stub_tool_callback_1}; + + ret = mcp_server_add_tool(&valid_tool); + zassert_equal(ret, 0, "Valid tool registration should succeed"); + zassert_equal(mcp_server_get_tool_count(), initial_count + 1, + "Tool count should increase by 1"); + + ret = mcp_server_remove_tool("test_tool_valid"); + zassert_equal(ret, 0, "Tool removal should succeed"); + zassert_equal(mcp_server_get_tool_count(), initial_count, + "Tool count should return to initial value"); +} + +ZTEST(mcp_server_tests, test_tool_registration_duplicate) +{ + int ret; + uint8_t initial_count = mcp_server_get_tool_count(); + + mcp_tool_record_t tool1 = {.metadata = { + .name = "duplicate_tool", + .input_schema = "{\"type\":\"object\"}", + }, + .callback = stub_tool_callback_1}; + mcp_tool_record_t tool2 = {.metadata = { + .name = "duplicate_tool", + .input_schema = "{\"type\":\"object\"}", + }, + .callback = stub_tool_callback_2}; + + ret = mcp_server_add_tool(&tool1); + zassert_equal(ret, 0, "First tool registration should succeed"); + zassert_equal(mcp_server_get_tool_count(), initial_count + 1, "Tool count should increase"); + + ret = mcp_server_add_tool(&tool2); + zassert_equal(ret, -EEXIST, "Duplicate tool registration should fail"); + zassert_equal(mcp_server_get_tool_count(), initial_count + 1, + "Tool count should not change on duplicate"); + + ret = mcp_server_remove_tool("duplicate_tool"); + zassert_equal(ret, 0, "Tool cleanup should succeed"); + zassert_equal(mcp_server_get_tool_count(), initial_count, "Tool count should be restored"); +} + +ZTEST(mcp_server_tests, test_tool_registration_edge_cases) +{ + int ret; + uint8_t initial_count = mcp_server_get_tool_count(); + + ret = mcp_server_add_tool(NULL); + zassert_equal(ret, -EINVAL, "NULL tool_record should fail"); + + mcp_tool_record_t empty_name_tool = { + .metadata = { + .name = "", + .input_schema = "{\"type\":\"object\"}", + }, + .callback = stub_tool_callback_1}; + + ret = mcp_server_add_tool(&empty_name_tool); + zassert_equal(ret, -EINVAL, "Empty tool name should fail"); + + mcp_tool_record_t null_callback_tool = { + .metadata = { + .name = "null_callback_tool", + .input_schema = "{\"type\":\"object\"}", + }, + .callback = NULL}; + + ret = mcp_server_add_tool(&null_callback_tool); + zassert_equal(ret, -EINVAL, "NULL callback should fail"); + + zassert_equal(mcp_server_get_tool_count(), initial_count, + "Tool count should not change after invalid attempts"); + + mcp_tool_record_t registry_tools[] = { + {.metadata = {.name = "registry_test_tool_1", + .input_schema = "{\"type\":\"object\"}"}, + .callback = stub_tool_callback_3}, + {.metadata = {.name = "registry_test_tool_2", + .input_schema = "{\"type\":\"object\"}"}, + .callback = stub_tool_callback_3}, + {.metadata = {.name = "registry_test_tool_3", + .input_schema = "{\"type\":\"object\"}"}, + .callback = stub_tool_callback_3}, + {.metadata = {.name = "registry_test_tool_4", + .input_schema = "{\"type\":\"object\"}"}, + .callback = stub_tool_callback_3}}; + + for (int i = 0; i < ARRAY_SIZE(registry_tools); i++) { + ret = mcp_server_add_tool(®istry_tools[i]); + zassert_equal(ret, 0, "Tool %d should register successfully", i + 1); + } + + zassert_equal(mcp_server_get_tool_count(), CONFIG_MCP_MAX_TOOLS, + "Registry should be at maximum capacity"); + + mcp_tool_record_t overflow_tool = {.metadata = { + .name = "registry_overflow_tool", + .input_schema = "{\"type\":\"object\"}", + }, + .callback = stub_tool_callback_3}; + + ret = mcp_server_add_tool(&overflow_tool); + zassert_equal(ret, -ENOSPC, "Registry overflow should fail"); + zassert_equal(mcp_server_get_tool_count(), CONFIG_MCP_MAX_TOOLS, + "Tool count should not change when registry is full"); + + for (int i = 0; i < ARRAY_SIZE(registry_tools); i++) { + mcp_server_remove_tool(registry_tools[i].metadata.name); + } + + zassert_equal(mcp_server_get_tool_count(), initial_count, + "Tool count should return to initial value"); +} + +ZTEST(mcp_server_tests, test_tool_removal) +{ + int ret; + uint8_t initial_count = mcp_server_get_tool_count(); + + mcp_tool_record_t test_tool = {.metadata = { + .name = "removal_test_tool", + .input_schema = "{\"type\":\"object\"}", + }, + .callback = stub_tool_callback_1}; + + ret = mcp_server_add_tool(&test_tool); + zassert_equal(ret, 0, "Tool addition should succeed"); + zassert_equal(mcp_server_get_tool_count(), initial_count + 1, "Tool count should increase"); + + ret = mcp_server_remove_tool("removal_test_tool"); + zassert_equal(ret, 0, "Tool removal should succeed"); + zassert_equal(mcp_server_get_tool_count(), initial_count, "Tool count should decrease"); + + ret = mcp_server_remove_tool("removal_test_tool"); + zassert_equal(ret, -ENOENT, "Removing non-existent tool should fail"); + + ret = mcp_server_remove_tool(NULL); + zassert_equal(ret, -EINVAL, "NULL tool name should fail"); + + ret = mcp_server_remove_tool(""); + zassert_equal(ret, -EINVAL, "Empty tool name should fail"); + + ret = mcp_server_remove_tool("never_existed_tool"); + zassert_equal(ret, -ENOENT, "Non-existent tool should fail"); +} + +ZTEST(mcp_server_tests, test_client_lifecycle) +{ + reset_transport_mock(); + + send_initialize_request(CLIENT_ID_LIFECYCLE_TEST, REQ_ID_LIFECYCLE_INITIALIZE); + zassert_equal(mcp_transport_queue_call_count, 1, "Initialize response should be sent"); + zassert_equal(mcp_transport_last_queued_msg.type, MCP_MSG_RESPONSE_INITIALIZE, + "Response should be initialize type"); + + reset_transport_mock(); + send_tools_list_request(CLIENT_ID_LIFECYCLE_TEST, REQ_ID_LIFECYCLE_TOOLS_INIT); + zassert_equal(mcp_transport_queue_call_count, 0, + "Tools/list should be rejected before client is initialized"); + + send_initialized_notification(CLIENT_ID_LIFECYCLE_TEST); + + reset_transport_mock(); + send_tools_list_request(CLIENT_ID_LIFECYCLE_TEST, REQ_ID_LIFECYCLE_TOOLS_READY); + zassert_equal(mcp_transport_queue_call_count, 1, + "Tools/list should succeed after initialization"); + zassert_equal(mcp_transport_last_queued_msg.type, MCP_MSG_RESPONSE_TOOLS_LIST, + "Response should be tools/list type"); + + send_client_shutdown(CLIENT_ID_LIFECYCLE_TEST); +} + +ZTEST(mcp_server_tests, test_client_shutdown) +{ + reset_transport_mock(); + + initialize_client_fully(CLIENT_ID_SHUTDOWN_TEST, REQ_ID_SHUTDOWN_INITIALIZE); + zassert_equal(mcp_transport_queue_call_count, 1, "Client initialization should succeed"); + + reset_transport_mock(); + send_tools_list_request(CLIENT_ID_SHUTDOWN_TEST, REQ_ID_SHUTDOWN_TOOLS_ACTIVE); + zassert_equal(mcp_transport_queue_call_count, 1, + "Tools/list should work for active client"); + + reset_transport_mock(); + send_client_shutdown(CLIENT_ID_SHUTDOWN_TEST); + zassert_equal(mcp_transport_queue_call_count, 0, "No response expected for shutdown"); + + reset_transport_mock(); + send_tools_list_request(CLIENT_ID_SHUTDOWN_TEST, REQ_ID_SHUTDOWN_TOOLS_DEAD); + zassert_equal(mcp_transport_queue_call_count, 0, + "Tools/list should be rejected for shutdown client"); + + reset_transport_mock(); + send_client_shutdown(CLIENT_ID_UNREGISTERED); + zassert_equal(mcp_transport_queue_call_count, 0, + "Shutdown of unregistered client should be handled gracefully"); +} + +ZTEST(mcp_server_tests, test_invalid_states) +{ + reset_transport_mock(); + + initialize_client_fully(CLIENT_ID_INVALID_STATE_TEST, REQ_ID_INVALID_INITIALIZE); + zassert_equal(mcp_transport_queue_call_count, 1, "Normal initialization should succeed"); + + reset_transport_mock(); + send_initialize_request(CLIENT_ID_INVALID_STATE_TEST, REQ_ID_INVALID_REINITIALIZE); + zassert_equal(mcp_transport_queue_call_count, 0, "Re-initialization should be rejected"); + + reset_transport_mock(); + send_initialized_notification(CLIENT_ID_UNREGISTERED); + zassert_equal(mcp_transport_queue_call_count, 0, + "Notification for unregistered client should be rejected"); + + reset_transport_mock(); + send_initialized_notification(CLIENT_ID_INVALID_STATE_TEST); + zassert_equal(mcp_transport_queue_call_count, 0, + "Duplicate initialized notification should be rejected"); + + send_client_shutdown(CLIENT_ID_INVALID_STATE_TEST); +} + +ZTEST(mcp_server_tests, test_multiple_client_lifecycle) +{ + reset_transport_mock(); + + initialize_client_fully(CLIENT_ID_MULTI_CLIENT_1, REQ_ID_MULTI_CLIENT_1_INIT); + initialize_client_fully(CLIENT_ID_MULTI_CLIENT_2, REQ_ID_MULTI_CLIENT_2_INIT); + initialize_client_fully(CLIENT_ID_MULTI_CLIENT_3, REQ_ID_MULTI_CLIENT_3_INIT); + + zassert_equal(mcp_transport_queue_call_count, 3, + "All 3 clients should initialize successfully"); + + reset_transport_mock(); + send_initialize_request(CLIENT_ID_MULTI_CLIENT_4, REQ_ID_MULTI_CLIENT_4_INIT_1); + zassert_equal(mcp_transport_queue_call_count, 0, + "4th client should be rejected when registry is full"); + + send_client_shutdown(CLIENT_ID_MULTI_CLIENT_1); + + reset_transport_mock(); + initialize_client_fully(CLIENT_ID_MULTI_CLIENT_4, REQ_ID_MULTI_CLIENT_4_INIT_2); + zassert_equal(mcp_transport_queue_call_count, 1, "4th client should succeed after cleanup"); + + send_client_shutdown(CLIENT_ID_MULTI_CLIENT_2); + send_client_shutdown(CLIENT_ID_MULTI_CLIENT_3); + send_client_shutdown(CLIENT_ID_MULTI_CLIENT_4); +} + +ZTEST(mcp_server_tests, test_unknown_message_type) +{ + int ret; + mcp_request_queue_msg_t msg; + char *test_data; + + reset_transport_mock(); + + test_data = (char *)mcp_alloc(32); + zassert_not_null(test_data, "Test data allocation should succeed"); + strcpy(test_data, "invalid_message_data"); + + msg.type = MCP_MSG_INVALID_TYPE; + msg.data = test_data; + + ret = k_msgq_put(&mcp_request_queue, &msg, K_NO_WAIT); + zassert_equal(ret, 0, "Invalid message queueing should succeed"); + + k_msleep(100); + + zassert_equal(mcp_transport_queue_call_count, 0, + "No response should be sent for unknown message type"); +} + +ZTEST(mcp_server_tests, test_server_double_init) +{ + int ret; + + ret = mcp_server_init(); + zassert_equal(ret, 0, "Second server init should succeed or handle gracefully"); +} + +ZTEST(mcp_server_tests, test_message_worker) +{ + int ret; + struct mcp_message_msg test_msg; + + memset(&test_msg, 0, sizeof(test_msg)); + + ret = k_msgq_put(&mcp_message_queue, &test_msg, K_NO_WAIT); + zassert_equal(ret, 0, "Message queueing should succeed"); + + k_msleep(100); +} + +ZTEST(mcp_server_tests, test_null_data_request) +{ + int ret; + mcp_request_queue_msg_t msg; + + reset_transport_mock(); + + msg.type = MCP_MSG_REQUEST_INITIALIZE; + msg.data = NULL; + + ret = k_msgq_put(&mcp_request_queue, &msg, K_NO_WAIT); + zassert_equal(ret, 0, "NULL data message queueing should succeed"); + + k_msleep(100); + + zassert_equal(mcp_transport_queue_call_count, 0, + "No response should be sent for NULL data pointer"); +} + +static void *mcp_server_tests_setup(void) +{ + int ret; + + ret = mcp_server_init(); + zassert_equal(ret, 0, "Server initialization should succeed"); + + ret = mcp_server_start(); + zassert_equal(ret, 0, "Server start should succeed"); + + k_msleep(100); + + return NULL; +} + +static void mcp_server_tests_before(void *fixture) +{ + ARG_UNUSED(fixture); + reset_transport_mock(); +} + +ZTEST_SUITE(mcp_server_tests, NULL, mcp_server_tests_setup, mcp_server_tests_before, NULL, NULL); diff --git a/tests/net/lib/mcp/testcase.yaml b/tests/net/lib/mcp/testcase.yaml new file mode 100644 index 0000000000000..287a5c4374774 --- /dev/null +++ b/tests/net/lib/mcp/testcase.yaml @@ -0,0 +1,5 @@ +tests: + net.mcp.server: + tags: mcp net + min_ram: 32 + timeout: 30 From a37a39fb411fc8afa2290de4117b5c49e71cef90 Mon Sep 17 00:00:00 2001 From: David Piskula Date: Thu, 6 Nov 2025 14:08:43 +0100 Subject: [PATCH 07/12] networking: tools call Implemented the tools/call method Signed-off-by: David Piskula --- subsys/net/lib/mcp/mcp_common.h | 4 +- subsys/net/lib/mcp/mcp_server.c | 441 ++++++++++++++++++++++++-------- tests/net/lib/mcp/prj.conf | 8 +- tests/net/lib/mcp/src/main.c | 206 ++++++++++++--- 4 files changed, 500 insertions(+), 159 deletions(-) diff --git a/subsys/net/lib/mcp/mcp_common.h b/subsys/net/lib/mcp/mcp_common.h index 8bf4d2ce5a7e0..4841f94a6fab4 100644 --- a/subsys/net/lib/mcp/mcp_common.h +++ b/subsys/net/lib/mcp/mcp_common.h @@ -125,7 +125,7 @@ typedef struct { uint32_t execution_token; uint32_t request_id; uint32_t client_id; - uint32_t worker_id; + k_tid_t worker_id; int64_t start_timestamp; int64_t cancel_timestamp; int64_t last_message_timestamp; @@ -135,7 +135,7 @@ typedef struct { typedef struct { mcp_execution_context_t executions[MCP_MAX_REQUESTS]; - struct k_mutex execution_mutex; + struct k_mutex registry_mutex; } mcp_execution_registry_t; #endif diff --git a/subsys/net/lib/mcp/mcp_server.c b/subsys/net/lib/mcp/mcp_server.c index 970aa89d13dfd..41416312aa375 100644 --- a/subsys/net/lib/mcp/mcp_server.c +++ b/subsys/net/lib/mcp/mcp_server.c @@ -19,7 +19,6 @@ LOG_MODULE_REGISTER(mcp_server, CONFIG_MCP_LOG_LEVEL); #define MCP_MESSAGE_QUEUE_SIZE 10 #define MCP_WORKER_PRIORITY 7 -/* Client lifecycle states */ typedef enum { MCP_LIFECYCLE_DEINITIALIZED = 0, MCP_LIFECYCLE_NEW, @@ -31,7 +30,7 @@ typedef enum { typedef struct { uint32_t client_id; mcp_lifecycle_state_t lifecycle_state; - uint32_t active_requests[MCP_MAX_REQUESTS]; + uint32_t active_requests[CONFIG_HTTP_SERVER_MAX_STREAMS]; uint8_t active_request_count; } mcp_client_context_t; @@ -43,6 +42,7 @@ typedef struct { static mcp_client_registry_t client_registry; static mcp_tool_registry_t tool_registry; +static mcp_execution_registry_t execution_registry; K_MSGQ_DEFINE(mcp_request_queue, sizeof(mcp_request_queue_msg_t), MCP_REQUEST_QUEUE_SIZE, 4); K_MSGQ_DEFINE(mcp_message_queue, sizeof(mcp_response_queue_msg_t), MCP_MESSAGE_QUEUE_SIZE, 4); @@ -53,13 +53,67 @@ K_THREAD_STACK_ARRAY_DEFINE(mcp_message_worker_stacks, MCP_MESSAGE_WORKERS, 2048 static struct k_thread mcp_request_workers[MCP_REQUEST_WORKERS]; static struct k_thread mcp_message_workers[MCP_MESSAGE_WORKERS]; +/* Helper functions */ + +static int find_client_index(uint32_t client_id) +{ + for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { + if (client_registry.clients[i].client_id == client_id) { + return i; + } + } + return -1; +} + +static int find_tool_index(const char *tool_name) +{ + for (int i = 0; i < CONFIG_MCP_MAX_TOOLS; i++) { + if (tool_registry.tools[i].metadata.name[0] != '\0' && + strncmp(tool_registry.tools[i].metadata.name, tool_name, + CONFIG_MCP_TOOL_NAME_MAX_LEN) == 0) { + return i; + } + } + return -1; +} + +static int find_available_client_slot(void) +{ + for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { + if (client_registry.clients[i].client_id == 0) { + return i; + } + } + return -1; +} + +static int find_available_tool_slot(void) +{ + for (int i = 0; i < CONFIG_MCP_MAX_TOOLS; i++) { + if (tool_registry.tools[i].metadata.name[0] == '\0') { + return i; + } + } + return -1; +} + +static int find_available_request_slot(int client_index) +{ + for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_STREAMS; i++) { + if (client_registry.clients[client_index].active_requests[i] == 0) { + return i; + } + } + return -1; +} + /* Must be called with registry_mutex held */ static void cleanup_client_registry_entry(int client_index) { client_registry.clients[client_index].client_id = 0; client_registry.clients[client_index].active_request_count = 0; - for (int i = 0; i < MCP_MAX_REQUESTS; i++) { + for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_STREAMS; i++) { client_registry.clients[client_index].active_requests[i] = 0; } @@ -67,10 +121,123 @@ static void cleanup_client_registry_entry(int client_index) client_registry.client_count--; } +static int register_new_client(uint32_t client_id) +{ + int slot_index; + + if (client_registry.client_count >= CONFIG_HTTP_SERVER_MAX_CLIENTS) { + return -ENOSPC; + } + + slot_index = find_available_client_slot(); + if (slot_index == -1) { + return -ENOSPC; + } + + client_registry.clients[slot_index].client_id = client_id; + client_registry.clients[slot_index].lifecycle_state = MCP_LIFECYCLE_NEW; + client_registry.clients[slot_index].active_request_count = 0; + client_registry.client_count++; + + return slot_index; +} + +#ifdef CONFIG_MCP_TOOLS_CAPABILITY +static uint32_t generate_execution_token(uint32_t request_id) +{ + /* Mocking the generation for the 1st phase of development. Security phase + * will replace this with UUID generation to make token guessing harder. + */ + return request_id; +} + +static int create_execution_context(mcp_tools_call_request_t *request, uint32_t *execution_token) +{ + int ret; + + if (request == NULL || execution_token == NULL) { + LOG_ERR("Invalid parameter(s)"); + return -EINVAL; + } + + ret = k_mutex_lock(&execution_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock execution registry: %d", ret); + return ret; + } + + *execution_token = generate_execution_token(request->request_id); + + bool found_slot = false; + + for (int i = 0; i < MCP_MAX_REQUESTS; i++) { + if (execution_registry.executions[i].execution_token == 0) { + mcp_execution_context_t *context = &execution_registry.executions[i]; + + context->execution_token = *execution_token; + context->request_id = request->request_id; + context->client_id = request->client_id; + context->worker_id = k_current_get(); + context->start_timestamp = k_uptime_get(); + context->cancel_timestamp = 0; + context->last_message_timestamp = 0; + context->worker_released = false; + context->execution_state = MCP_EXEC_ACTIVE; + + found_slot = true; + break; + } + } + + k_mutex_unlock(&execution_registry.registry_mutex); + + if (!found_slot) { + LOG_ERR("Execution registry full"); + return -ENOENT; + } + + return 0; +} + +static int destroy_execution_context(uint32_t execution_token) +{ + int ret; + + ret = k_mutex_lock(&execution_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock execution registry: %d", ret); + return ret; + } + + bool found_token = false; + + for (int i = 0; i < MCP_MAX_REQUESTS; i++) { + if (execution_registry.executions[i].execution_token == execution_token) { + mcp_execution_context_t *context = &execution_registry.executions[i]; + + memset(context, 0, sizeof(*context)); + found_token = true; + break; + } + } + + k_mutex_unlock(&execution_registry.registry_mutex); + + if (!found_token) { + LOG_ERR("Unknown execution token"); + return -ENOENT; + } + + return 0; +} +#endif + +/* Message handlers */ + static int handle_system_message(mcp_system_msg_t *system_msg) { int ret; - int client_index = -1; + int client_index; LOG_DBG("Processing system request"); @@ -82,21 +249,16 @@ static int handle_system_message(mcp_system_msg_t *system_msg) return ret; } - for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { - if (client_registry.clients[i].client_id == system_msg->client_id) { - client_index = i; - client_registry.clients[i].lifecycle_state = - MCP_LIFECYCLE_DEINITIALIZING; - break; - } - } - + client_index = find_client_index(system_msg->client_id); if (client_index == -1) { LOG_ERR("Client not registered"); k_mutex_unlock(&client_registry.registry_mutex); return -ENOENT; } + client_registry.clients[client_index].lifecycle_state = + MCP_LIFECYCLE_DEINITIALIZING; + /* TODO: Cancel active tool executions */ cleanup_client_registry_entry(client_index); k_mutex_unlock(&client_registry.registry_mutex); @@ -114,7 +276,7 @@ static int handle_initialize_request(mcp_initialize_request_t *request) { mcp_initialize_response_t *response_data; mcp_response_queue_msg_t response; - int client_index = -1; + int client_index; int ret; LOG_DBG("Processing initialize request"); @@ -125,31 +287,13 @@ static int handle_initialize_request(mcp_initialize_request_t *request) return ret; } - /* Search for existing client */ - for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { - if (client_registry.clients[i].client_id == request->client_id) { - client_index = i; - break; - } - } - - /* Register new client if needed */ + client_index = find_client_index(request->client_id); if (client_index == -1) { - if (client_registry.client_count >= CONFIG_HTTP_SERVER_MAX_CLIENTS) { + client_index = register_new_client(request->client_id); + if (client_index < 0) { LOG_ERR("Client registry full"); k_mutex_unlock(&client_registry.registry_mutex); - return -ENOSPC; - } - - for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { - if (client_registry.clients[i].client_id == 0) { - client_registry.clients[i].client_id = request->client_id; - client_registry.clients[i].lifecycle_state = MCP_LIFECYCLE_NEW; - client_registry.clients[i].active_request_count = 0; - client_index = i; - client_registry.client_count++; - break; - } + return client_index; } } @@ -190,59 +334,24 @@ static int handle_initialize_request(mcp_initialize_request_t *request) } #ifdef CONFIG_MCP_TOOLS_CAPABILITY -static int handle_tools_list_request(mcp_tools_list_request_t *request) +static int validate_client_for_tools_request(uint32_t client_id, int *client_index) { - mcp_tools_list_response_t *response_data; - mcp_response_queue_msg_t response; - int client_index = -1; - int ret; - - LOG_DBG("Processing tools list request"); - - /* Check client state in single lock operation */ - ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); - if (ret != 0) { - LOG_ERR("Failed to lock client registry mutex: %d", ret); - return ret; - } - - for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { - if (client_registry.clients[i].client_id == request->client_id) { - client_index = i; - break; - } - } - - if (client_index == -1) { - k_mutex_unlock(&client_registry.registry_mutex); + *client_index = find_client_index(client_id); + if (*client_index == -1) { return -ENOENT; } - if (client_registry.clients[client_index].lifecycle_state != MCP_LIFECYCLE_INITIALIZED) { - k_mutex_unlock(&client_registry.registry_mutex); + if (client_registry.clients[*client_index].lifecycle_state != MCP_LIFECYCLE_INITIALIZED) { return -EPERM; } - k_mutex_unlock(&client_registry.registry_mutex); - - response_data = (mcp_tools_list_response_t *)mcp_alloc(sizeof(mcp_tools_list_response_t)); - if (!response_data) { - LOG_ERR("Failed to allocate response"); - return -ENOMEM; - } - - response_data->request_id = request->request_id; - - ret = k_mutex_lock(&tool_registry.registry_mutex, K_FOREVER); - if (ret != 0) { - LOG_ERR("Failed to lock tool registry"); - mcp_free(response_data); - return ret; - } + return 0; +} +static int copy_tool_metadata_to_response(mcp_tools_list_response_t *response_data) +{ response_data->tool_count = tool_registry.tool_count; - /* Copy tool metadata */ for (int i = 0; i < tool_registry.tool_count; i++) { strncpy(response_data->tools[i].name, tool_registry.tools[i].metadata.name, CONFIG_MCP_TOOL_NAME_MAX_LEN - 1); @@ -289,6 +398,47 @@ static int handle_tools_list_request(mcp_tools_list_request_t *request) #endif } + return 0; +} + +static int handle_tools_list_request(mcp_tools_list_request_t *request) +{ + mcp_tools_list_response_t *response_data; + mcp_response_queue_msg_t response; + int client_index; + int ret; + + LOG_DBG("Processing tools list request"); + + ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock client registry mutex: %d", ret); + return ret; + } + + ret = validate_client_for_tools_request(request->client_id, &client_index); + k_mutex_unlock(&client_registry.registry_mutex); + + if (ret != 0) { + return ret; + } + + response_data = (mcp_tools_list_response_t *)mcp_alloc(sizeof(mcp_tools_list_response_t)); + if (!response_data) { + LOG_ERR("Failed to allocate response"); + return -ENOMEM; + } + + response_data->request_id = request->request_id; + + ret = k_mutex_lock(&tool_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock tool registry"); + mcp_free(response_data); + return ret; + } + + copy_tool_metadata_to_response(response_data); k_mutex_unlock(&tool_registry.registry_mutex); response.type = MCP_MSG_RESPONSE_TOOLS_LIST; @@ -306,16 +456,92 @@ static int handle_tools_list_request(mcp_tools_list_request_t *request) static int handle_tools_call_request(mcp_tools_call_request_t *request) { - LOG_DBG("Tool call request for client %u", request->client_id); - /* TODO: Implement tool execution */ + int ret; + int tool_index; + int client_index; + int next_active_request_index; + mcp_tool_callback_t callback; + uint32_t execution_token; + + LOG_DBG("Processing tools call request"); + + ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock client registry mutex: %d", ret); + return ret; + } + + ret = validate_client_for_tools_request(request->client_id, &client_index); + if (ret != 0) { + k_mutex_unlock(&client_registry.registry_mutex); + return ret; + } + + next_active_request_index = find_available_request_slot(client_index); + if (next_active_request_index == -1) { + LOG_ERR("No available request slot for client"); + k_mutex_unlock(&client_registry.registry_mutex); + return -ENOSPC; + } + + client_registry.clients[client_index].active_requests[next_active_request_index] = + request->request_id; + client_registry.clients[client_index].active_request_count++; + + k_mutex_unlock(&client_registry.registry_mutex); + + ret = k_mutex_lock(&tool_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock tool registry: %d", ret); + goto cleanup_active_request; + } + + tool_index = find_tool_index(request->name); + if (tool_index == -1) { + LOG_ERR("Tool '%s' not found", request->name); + k_mutex_unlock(&tool_registry.registry_mutex); + ret = -ENOENT; + goto cleanup_active_request; + } + + callback = tool_registry.tools[tool_index].callback; + k_mutex_unlock(&tool_registry.registry_mutex); + + ret = create_execution_context(request, &execution_token); + if (ret != 0) { + LOG_ERR("Failed to create execution context: %d", ret); + goto cleanup_active_request; + } + + ret = callback(request->arguments, execution_token); + if (ret != 0) { + LOG_ERR("Tool callback failed: %d", ret); + destroy_execution_context(execution_token); + goto cleanup_active_request; + } + return 0; + +cleanup_active_request: + ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock client registry mutex: %d. Client registry is broken.", + ret); + return ret; + } + + client_registry.clients[client_index].active_requests[next_active_request_index] = 0; + client_registry.clients[client_index].active_request_count--; + + k_mutex_unlock(&client_registry.registry_mutex); + return ret; } #endif static int handle_notification(mcp_client_notification_t *notification) { int ret; - int client_index = -1; + int client_index; LOG_DBG("Processing notification"); @@ -325,13 +551,7 @@ static int handle_notification(mcp_client_notification_t *notification) return ret; } - for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { - if (client_registry.clients[i].client_id == notification->client_id) { - client_index = i; - break; - } - } - + client_index = find_client_index(notification->client_id); if (client_index == -1) { LOG_ERR("Client not found"); k_mutex_unlock(&client_registry.registry_mutex); @@ -362,6 +582,8 @@ static int handle_notification(mcp_client_notification_t *notification) return 0; } +/* Worker threads */ + static void mcp_request_worker(void *arg1, void *arg2, void *arg3) { mcp_request_queue_msg_t request; @@ -468,6 +690,8 @@ static void mcp_message_worker(void *arg1, void *arg2, void *arg3) } } +/* Public API functions */ + int mcp_server_init(void) { int ret; @@ -486,9 +710,16 @@ int mcp_server_init(void) return ret; } + ret = k_mutex_init(&execution_registry.registry_mutex); + if (ret != 0) { + LOG_ERR("Failed to init tool mutex: %d", ret); + return ret; + } + memset(&client_registry.clients, 0, sizeof(client_registry.clients)); tool_registry.tool_count = 0; memset(&tool_registry.tools, 0, sizeof(tool_registry.tools)); + memset(&execution_registry.executions, 0, sizeof(execution_registry.executions)); LOG_INF("MCP Server initialized"); return 0; @@ -549,7 +780,7 @@ int mcp_queue_response(void) int mcp_server_add_tool(const mcp_tool_record_t *tool_record) { int ret; - int available_slot = -1; + int available_slot; if (!tool_record || !tool_record->metadata.name[0] || !tool_record->callback) { LOG_ERR("Invalid tool record"); @@ -562,21 +793,13 @@ int mcp_server_add_tool(const mcp_tool_record_t *tool_record) return ret; } - /* Find available slot and check for duplicates */ - for (int i = 0; i < CONFIG_MCP_MAX_TOOLS; i++) { - if (tool_registry.tools[i].metadata.name[0] == '\0' && available_slot == -1) { - available_slot = i; - } - - if (tool_registry.tools[i].metadata.name[0] != '\0' && - strncmp(tool_registry.tools[i].metadata.name, tool_record->metadata.name, - CONFIG_MCP_TOOL_NAME_MAX_LEN) == 0) { - LOG_ERR("Tool '%s' already exists", tool_record->metadata.name); - k_mutex_unlock(&tool_registry.registry_mutex); - return -EEXIST; - } + if (find_tool_index(tool_record->metadata.name) != -1) { + LOG_ERR("Tool '%s' already exists", tool_record->metadata.name); + k_mutex_unlock(&tool_registry.registry_mutex); + return -EEXIST; } + available_slot = find_available_tool_slot(); if (available_slot == -1) { LOG_ERR("Tool registry full"); k_mutex_unlock(&tool_registry.registry_mutex); @@ -595,7 +818,7 @@ int mcp_server_add_tool(const mcp_tool_record_t *tool_record) int mcp_server_remove_tool(const char *tool_name) { int ret; - int tool_index = -1; + int tool_index; if (!tool_name || !tool_name[0]) { LOG_ERR("Invalid tool name"); @@ -608,15 +831,7 @@ int mcp_server_remove_tool(const char *tool_name) return ret; } - for (int i = 0; i < CONFIG_MCP_MAX_TOOLS; i++) { - if (tool_registry.tools[i].metadata.name[0] != '\0' && - strncmp(tool_registry.tools[i].metadata.name, tool_name, - CONFIG_MCP_TOOL_NAME_MAX_LEN) == 0) { - tool_index = i; - break; - } - } - + tool_index = find_tool_index(tool_name); if (tool_index == -1) { k_mutex_unlock(&tool_registry.registry_mutex); LOG_ERR("Tool '%s' not found", tool_name); diff --git a/tests/net/lib/mcp/prj.conf b/tests/net/lib/mcp/prj.conf index 6ef2a4a042a73..e042512570ccf 100644 --- a/tests/net/lib/mcp/prj.conf +++ b/tests/net/lib/mcp/prj.conf @@ -6,6 +6,8 @@ CONFIG_MCP_TOOL_DESC=y CONFIG_MCP_SERVER_INFO_TITLE=y CONFIG_MCP_SERVER_INFO_INSTRUCTIONS=y CONFIG_MCP_TOOL_DESC=y +CONFIG_MCP_TOOL_TITLE=y +CONFIG_MCP_TOOL_OUTPUT_SCHEMA=y CONFIG_MCP_TOOL_NAME_MAX_LEN=32 CONFIG_MCP_TOOL_SCHEMA_MAX_LEN=512 CONFIG_MCP_TOOL_INPUT_ARGS_MAX_LEN=512 @@ -17,11 +19,9 @@ CONFIG_NETWORKING=y CONFIG_HTTP_SERVER=y CONFIG_JSON_LIBRARY=y -# Enable kernel heap for k_malloc/k_free -CONFIG_HEAP_MEM_POOL_SIZE=16384 +CONFIG_HEAP_MEM_POOL_SIZE=65536 CONFIG_KERNEL_MEM_POOL=y -# Increase stack sizes to prevent stack overflow CONFIG_MAIN_STACK_SIZE=4096 CONFIG_SYSTEM_WORKQUEUE_STACK_SIZE=4096 -CONFIG_TEST_EXTRA_STACK_SIZE=2048 +CONFIG_TEST_EXTRA_STACK_SIZE=8192 diff --git a/tests/net/lib/mcp/src/main.c b/tests/net/lib/mcp/src/main.c index e853216143e8f..e01f2c81ff9b7 100644 --- a/tests/net/lib/mcp/src/main.c +++ b/tests/net/lib/mcp/src/main.c @@ -17,6 +17,8 @@ extern struct k_msgq mcp_message_queue; extern int mcp_transport_queue_call_count; extern mcp_response_queue_msg_t mcp_transport_last_queued_msg; +static int tool_execution_count; + #ifdef CONFIG_ZTEST extern uint8_t mcp_server_get_client_count(void); extern uint8_t mcp_server_get_tool_count(void); @@ -82,6 +84,48 @@ static void reset_transport_mock(void) memset(&mcp_transport_last_queued_msg, 0, sizeof(mcp_transport_last_queued_msg)); } +static int test_execution_tool_callback(const char *params, uint32_t execution_token) +{ + tool_execution_count++; + + printk("Tool execution callback executed! Count: %d, Token: %u, Arguments: %s\n", + tool_execution_count, execution_token, params ? params : "(null)"); + + return 0; +} + +static void send_tools_call_request(uint32_t client_id, uint32_t request_id, const char *tool_name, + const char *arguments) +{ + int ret; + mcp_request_queue_msg_t msg; + mcp_tools_call_request_t *tools_req; + + tools_req = (mcp_tools_call_request_t *)mcp_alloc(sizeof(mcp_tools_call_request_t)); + zassert_not_null(tools_req, "Tools call request allocation failed"); + + tools_req->request_id = request_id; + tools_req->client_id = client_id; + + strncpy(tools_req->name, tool_name, CONFIG_MCP_TOOL_NAME_MAX_LEN - 1); + tools_req->name[CONFIG_MCP_TOOL_NAME_MAX_LEN - 1] = '\0'; + + if (arguments) { + strncpy(tools_req->arguments, arguments, CONFIG_MCP_TOOL_INPUT_ARGS_MAX_LEN - 1); + tools_req->arguments[CONFIG_MCP_TOOL_INPUT_ARGS_MAX_LEN - 1] = '\0'; + } else { + tools_req->arguments[0] = '\0'; + } + + msg.type = MCP_MSG_REQUEST_TOOLS_CALL; + msg.data = tools_req; + + ret = k_msgq_put(&mcp_request_queue, &msg, K_NO_WAIT); + zassert_equal(ret, 0, "Tools call request queueing failed"); + + k_msleep(100); +} + static void send_initialize_request(uint32_t client_id, uint32_t request_id) { int ret; @@ -213,22 +257,25 @@ ZTEST(mcp_server_tests, test_tools_list_response) .output_schema = "{\"type\":\"string\"}", #endif }, - .callback = stub_tool_callback_1}; + .callback = stub_tool_callback_1 + }; - mcp_tool_record_t test_tool2 = {.metadata = { - .name = "test_tool_2", - .input_schema = "{\"type\":\"array\"}", + mcp_tool_record_t test_tool2 = { + .metadata = { + .name = "test_tool_2", + .input_schema = "{\"type\":\"array\"}", #ifdef CONFIG_MCP_TOOL_DESC - .description = "Second test tool", + .description = "Second test tool", #endif #ifdef CONFIG_MCP_TOOL_TITLE - .title = "Test Tool Two", + .title = "Test Tool Two", #endif #ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA - .output_schema = "{\"type\":\"number\"}", + .output_schema = "{\"type\":\"number\"}", #endif - }, - .callback = stub_tool_callback_2}; + }, + .callback = stub_tool_callback_2 + }; ret = mcp_server_add_tool(&test_tool1); zassert_equal(ret, 0, "Test tool 1 should register successfully"); @@ -347,20 +394,22 @@ ZTEST(mcp_server_tests, test_tool_registration_valid) int ret; uint8_t initial_count = mcp_server_get_tool_count(); - mcp_tool_record_t valid_tool = {.metadata = { - .name = "test_tool_valid", - .input_schema = "{\"type\":\"object\"}", + mcp_tool_record_t valid_tool = { + .metadata = { + .name = "test_tool_valid", + .input_schema = "{\"type\":\"object\"}", #ifdef CONFIG_MCP_TOOL_DESC - .description = "Test tool description", + .description = "Test tool description", #endif #ifdef CONFIG_MCP_TOOL_TITLE - .title = "Test Tool", + .title = "Test Tool", #endif #ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA - .output_schema = "{\"type\":\"object\"}", + .output_schema = "{\"type\":\"object\"}", #endif - }, - .callback = stub_tool_callback_1}; + }, + .callback = stub_tool_callback_1 + }; ret = mcp_server_add_tool(&valid_tool); zassert_equal(ret, 0, "Valid tool registration should succeed"); @@ -378,16 +427,20 @@ ZTEST(mcp_server_tests, test_tool_registration_duplicate) int ret; uint8_t initial_count = mcp_server_get_tool_count(); - mcp_tool_record_t tool1 = {.metadata = { - .name = "duplicate_tool", - .input_schema = "{\"type\":\"object\"}", - }, - .callback = stub_tool_callback_1}; - mcp_tool_record_t tool2 = {.metadata = { - .name = "duplicate_tool", - .input_schema = "{\"type\":\"object\"}", - }, - .callback = stub_tool_callback_2}; + mcp_tool_record_t tool1 = { + .metadata = { + .name = "duplicate_tool", + .input_schema = "{\"type\":\"object\"}", + }, + .callback = stub_tool_callback_1 + }; + mcp_tool_record_t tool2 = { + .metadata = { + .name = "duplicate_tool", + .input_schema = "{\"type\":\"object\"}", + }, + .callback = stub_tool_callback_2 + }; ret = mcp_server_add_tool(&tool1); zassert_equal(ret, 0, "First tool registration should succeed"); @@ -416,7 +469,8 @@ ZTEST(mcp_server_tests, test_tool_registration_edge_cases) .name = "", .input_schema = "{\"type\":\"object\"}", }, - .callback = stub_tool_callback_1}; + .callback = stub_tool_callback_1 + }; ret = mcp_server_add_tool(&empty_name_tool); zassert_equal(ret, -EINVAL, "Empty tool name should fail"); @@ -426,7 +480,8 @@ ZTEST(mcp_server_tests, test_tool_registration_edge_cases) .name = "null_callback_tool", .input_schema = "{\"type\":\"object\"}", }, - .callback = NULL}; + .callback = NULL + }; ret = mcp_server_add_tool(&null_callback_tool); zassert_equal(ret, -EINVAL, "NULL callback should fail"); @@ -446,7 +501,8 @@ ZTEST(mcp_server_tests, test_tool_registration_edge_cases) .callback = stub_tool_callback_3}, {.metadata = {.name = "registry_test_tool_4", .input_schema = "{\"type\":\"object\"}"}, - .callback = stub_tool_callback_3}}; + .callback = stub_tool_callback_3} + }; for (int i = 0; i < ARRAY_SIZE(registry_tools); i++) { ret = mcp_server_add_tool(®istry_tools[i]); @@ -456,11 +512,13 @@ ZTEST(mcp_server_tests, test_tool_registration_edge_cases) zassert_equal(mcp_server_get_tool_count(), CONFIG_MCP_MAX_TOOLS, "Registry should be at maximum capacity"); - mcp_tool_record_t overflow_tool = {.metadata = { - .name = "registry_overflow_tool", - .input_schema = "{\"type\":\"object\"}", - }, - .callback = stub_tool_callback_3}; + mcp_tool_record_t overflow_tool = { + .metadata = { + .name = "registry_overflow_tool", + .input_schema = "{\"type\":\"object\"}", + }, + .callback = stub_tool_callback_3 + }; ret = mcp_server_add_tool(&overflow_tool); zassert_equal(ret, -ENOSPC, "Registry overflow should fail"); @@ -480,11 +538,13 @@ ZTEST(mcp_server_tests, test_tool_removal) int ret; uint8_t initial_count = mcp_server_get_tool_count(); - mcp_tool_record_t test_tool = {.metadata = { - .name = "removal_test_tool", - .input_schema = "{\"type\":\"object\"}", - }, - .callback = stub_tool_callback_1}; + mcp_tool_record_t test_tool = { + .metadata = { + .name = "removal_test_tool", + .input_schema = "{\"type\":\"object\"}", + }, + .callback = stub_tool_callback_1 + }; ret = mcp_server_add_tool(&test_tool); zassert_equal(ret, 0, "Tool addition should succeed"); @@ -675,6 +735,72 @@ ZTEST(mcp_server_tests, test_null_data_request) "No response should be sent for NULL data pointer"); } +ZTEST(mcp_server_tests, test_tools_call) +{ + int ret; + uint8_t initial_tool_count = mcp_server_get_tool_count(); + + tool_execution_count = 0; + + reset_transport_mock(); + + mcp_tool_record_t execution_tool = { + .metadata = { + .name = "execution_test_tool", + .input_schema = "{\"type\":\"object\",\"properties\":" + "{\"param1\":{\"type\":\"string\"}}}", +#ifdef CONFIG_MCP_TOOL_DESC + .description = "Tool for testing execution", +#endif +#ifdef CONFIG_MCP_TOOL_TITLE + .title = "Execution Test Tool", +#endif +#ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA + .output_schema = "{\"type\":\"string\"}", +#endif + }, + .callback = test_execution_tool_callback + }; + + ret = mcp_server_add_tool(&execution_tool); + zassert_equal(ret, 0, "Execution test tool should register successfully"); + zassert_equal(mcp_server_get_tool_count(), initial_tool_count + 1, + "Tool count should increase"); + + initialize_client_fully(CLIENT_ID_EDGE_CASE_TEST, REQ_ID_EDGE_CASE_INITIALIZE); + + reset_transport_mock(); + send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, REQ_ID_EDGE_CASE_TOOLS_LIST, + "execution_test_tool", "{\"param1\":\"test_value\"}"); + + zassert_equal(tool_execution_count, 1, "Tool callback should have been executed once"); + + int previous_count = tool_execution_count; + + reset_transport_mock(); + send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, REQ_ID_EDGE_CASE_TOOLS_LIST + 1, + "non_existent_tool", "{}"); + + zassert_equal(tool_execution_count, previous_count, + "Tool callback should not be executed for non-existent tool"); + + previous_count = tool_execution_count; + reset_transport_mock(); + send_tools_call_request(CLIENT_ID_UNREGISTERED, REQ_ID_EDGE_CASE_UNREGISTERED, + "execution_test_tool", "{}"); + + zassert_equal(tool_execution_count, previous_count, + "Tool callback should not be executed for unregistered client"); + + mcp_server_remove_tool("execution_test_tool"); + zassert_equal(mcp_server_get_tool_count(), initial_tool_count, + "Tool count should return to initial value"); + + send_client_shutdown(CLIENT_ID_EDGE_CASE_TEST); + + printk("Tool execution test completed successfully\n"); +} + static void *mcp_server_tests_setup(void) { int ret; From e614ca39dd3961ee43e19988ab3f62e03d7ce0dc Mon Sep 17 00:00:00 2001 From: David Piskula Date: Fri, 7 Nov 2025 16:30:25 +0100 Subject: [PATCH 08/12] networking: tool response Implemented tool response processing and testing Signed-off-by: David Piskula --- include/zephyr/net/mcp/mcp_server.h | 23 +- subsys/net/lib/mcp/mcp_common.h | 6 + subsys/net/lib/mcp/mcp_server.c | 259 ++++++++-- subsys/net/lib/mcp/mcp_transport.c | 57 ++- subsys/net/lib/mcp/mcp_transport.h | 2 +- tests/net/lib/mcp/src/main.c | 723 +++++++++++++++++++++++++--- 6 files changed, 961 insertions(+), 109 deletions(-) diff --git a/include/zephyr/net/mcp/mcp_server.h b/include/zephyr/net/mcp/mcp_server.h index 231ac1a24fca4..532264fb20932 100644 --- a/include/zephyr/net/mcp/mcp_server.h +++ b/include/zephyr/net/mcp/mcp_server.h @@ -19,6 +19,14 @@ extern "C" { #endif +typedef enum { +#ifdef CONFIG_MCP_TOOLS_CAPABILITY + MCP_USR_TOOL_RESPONSE, + MCP_USR_TOOL_NOTIFICATION, +#endif + MCP_USR_GENERIC_RESPONSE +} mcp_app_msg_type_t; + #ifdef CONFIG_MCP_TOOLS_CAPABILITY /** * @brief Tool metadata structure @@ -55,9 +63,11 @@ typedef struct mcp_tool_record { } mcp_tool_record_t; #endif -struct mcp_message_msg { - uint32_t token; -}; +typedef struct mcp_user_message { + mcp_app_msg_type_t type; + int length; + void *data; +} mcp_app_message_t; /** * @brief Initialize the MCP Server @@ -73,12 +83,7 @@ int mcp_server_init(void); */ int mcp_server_start(void); -/** - * @brief Queue a response for delivery - * - * @return 0 on success, negative errno on failure - */ -int mcp_queue_response(void); +int mcp_server_submit_app_message(const mcp_app_message_t *user_msg, uint32_t execution_token); #ifdef CONFIG_MCP_TOOLS_CAPABILITY /** diff --git a/subsys/net/lib/mcp/mcp_common.h b/subsys/net/lib/mcp/mcp_common.h index 4841f94a6fab4..74eb5421852df 100644 --- a/subsys/net/lib/mcp/mcp_common.h +++ b/subsys/net/lib/mcp/mcp_common.h @@ -91,6 +91,7 @@ typedef struct mcp_tools_list_response { typedef struct mcp_tools_call_response { uint32_t request_id; + int length; char result[CONFIG_MCP_TOOL_RESULT_MAX_LEN]; } mcp_tools_call_response_t; #endif @@ -109,6 +110,11 @@ typedef struct mcp_request_queue_msg { void *data; } mcp_request_queue_msg_t; +typedef struct mcp_transport_queue_msg { + mcp_queue_msg_type_t type; + void *data; +} mcp_transport_queue_msg_t; + typedef struct mcp_response_queue_msg { mcp_queue_msg_type_t type; void *data; diff --git a/subsys/net/lib/mcp/mcp_server.c b/subsys/net/lib/mcp/mcp_server.c index 41416312aa375..7cfa7bbe075e4 100644 --- a/subsys/net/lib/mcp/mcp_server.c +++ b/subsys/net/lib/mcp/mcp_server.c @@ -65,6 +65,16 @@ static int find_client_index(uint32_t client_id) return -1; } +static int find_request_index(int client_index, uint32_t request_id) +{ + for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_STREAMS; i++) { + if (client_registry.clients[client_index].active_requests[i] == request_id) { + return i; + } + } + return -1; +} + static int find_tool_index(const char *tool_name) { for (int i = 0; i < CONFIG_MCP_MAX_TOOLS; i++) { @@ -77,6 +87,16 @@ static int find_tool_index(const char *tool_name) return -1; } +static int find_token_index(const uint32_t execution_token) +{ + for (int i = 0; i < MCP_MAX_REQUESTS; i++) { + if (execution_registry.executions[i].execution_token == execution_token) { + return i; + } + } + return -1; +} + static int find_available_client_slot(void) { for (int i = 0; i < CONFIG_HTTP_SERVER_MAX_CLIENTS; i++) { @@ -151,11 +171,12 @@ static uint32_t generate_execution_token(uint32_t request_id) return request_id; } -static int create_execution_context(mcp_tools_call_request_t *request, uint32_t *execution_token) +static int create_execution_context(mcp_tools_call_request_t *request, uint32_t *execution_token, + int *execution_token_index) { int ret; - if (request == NULL || execution_token == NULL) { + if (request == NULL || execution_token == NULL || execution_token_index == NULL) { LOG_ERR("Invalid parameter(s)"); return -EINVAL; } @@ -185,6 +206,7 @@ static int create_execution_context(mcp_tools_call_request_t *request, uint32_t context->execution_state = MCP_EXEC_ACTIVE; found_slot = true; + *execution_token_index = i; break; } } @@ -199,6 +221,23 @@ static int create_execution_context(mcp_tools_call_request_t *request, uint32_t return 0; } +static int set_worker_released_execution_context(int execution_token_index) +{ + int ret; + + ret = k_mutex_lock(&execution_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock execution registry: %d", ret); + + return ret; + } + + execution_registry.executions[execution_token_index].worker_released = true; + + k_mutex_unlock(&execution_registry.registry_mutex); + return 0; +} + static int destroy_execution_context(uint32_t execution_token) { int ret; @@ -275,11 +314,11 @@ static int handle_system_message(mcp_system_msg_t *system_msg) static int handle_initialize_request(mcp_initialize_request_t *request) { mcp_initialize_response_t *response_data; - mcp_response_queue_msg_t response; int client_index; int ret; LOG_DBG("Processing initialize request"); + /* TODO: Handle sending error responses to client */ ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); if (ret != 0) { @@ -320,10 +359,7 @@ static int handle_initialize_request(mcp_initialize_request_t *request) response_data->capabilities |= MCP_TOOLS; #endif - response.type = MCP_MSG_RESPONSE_INITIALIZE; - response.data = response_data; - - ret = mcp_transport_queue_response(&response); + ret = mcp_transport_queue_response(MCP_MSG_RESPONSE_INITIALIZE, response_data); if (ret != 0) { LOG_ERR("Failed to queue response: %d", ret); mcp_free(response_data); @@ -404,10 +440,10 @@ static int copy_tool_metadata_to_response(mcp_tools_list_response_t *response_da static int handle_tools_list_request(mcp_tools_list_request_t *request) { mcp_tools_list_response_t *response_data; - mcp_response_queue_msg_t response; int client_index; int ret; + /* TODO: Handle sending error responses to client */ LOG_DBG("Processing tools list request"); ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); @@ -441,10 +477,7 @@ static int handle_tools_list_request(mcp_tools_list_request_t *request) copy_tool_metadata_to_response(response_data); k_mutex_unlock(&tool_registry.registry_mutex); - response.type = MCP_MSG_RESPONSE_TOOLS_LIST; - response.data = response_data; - - ret = mcp_transport_queue_response(&response); + ret = mcp_transport_queue_response(MCP_MSG_RESPONSE_TOOLS_LIST, response_data); if (ret != 0) { LOG_ERR("Failed to queue response: %d", ret); mcp_free(response_data); @@ -459,6 +492,7 @@ static int handle_tools_call_request(mcp_tools_call_request_t *request) int ret; int tool_index; int client_index; + int execution_token_index; int next_active_request_index; mcp_tool_callback_t callback; uint32_t execution_token; @@ -507,7 +541,7 @@ static int handle_tools_call_request(mcp_tools_call_request_t *request) callback = tool_registry.tools[tool_index].callback; k_mutex_unlock(&tool_registry.registry_mutex); - ret = create_execution_context(request, &execution_token); + ret = create_execution_context(request, &execution_token, &execution_token_index); if (ret != 0) { LOG_ERR("Failed to create execution context: %d", ret); goto cleanup_active_request; @@ -520,9 +554,18 @@ static int handle_tools_call_request(mcp_tools_call_request_t *request) goto cleanup_active_request; } + ret = set_worker_released_execution_context(execution_token_index); + if (ret != 0) { + LOG_ERR("Setting worker released to true failed: %d", ret); + /* TODO: Cancel tool execution */ + destroy_execution_context(execution_token); + goto cleanup_active_request; + } + return 0; cleanup_active_request: + int final_ret = ret; ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); if (ret != 0) { LOG_ERR("Failed to lock client registry mutex: %d. Client registry is broken.", @@ -534,7 +577,7 @@ static int handle_tools_call_request(mcp_tools_call_request_t *request) client_registry.clients[client_index].active_request_count--; k_mutex_unlock(&client_registry.registry_mutex); - return ret; + return final_ret; } #endif @@ -617,49 +660,52 @@ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) } case MCP_MSG_REQUEST_INITIALIZE: { - mcp_initialize_request_t *req = (mcp_initialize_request_t *)request.data; + mcp_initialize_request_t *init_request = + (mcp_initialize_request_t *)request.data; - ret = handle_initialize_request(req); + ret = handle_initialize_request(init_request); if (ret != 0) { LOG_ERR("Initialize request failed: %d", ret); } - mcp_free(req); + mcp_free(init_request); break; } #ifdef CONFIG_MCP_TOOLS_CAPABILITY case MCP_MSG_REQUEST_TOOLS_LIST: { - mcp_tools_list_request_t *req = (mcp_tools_list_request_t *)request.data; + mcp_tools_list_request_t *tools_list_request = + (mcp_tools_list_request_t *)request.data; - ret = handle_tools_list_request(req); + ret = handle_tools_list_request(tools_list_request); if (ret != 0) { LOG_ERR("Tools list request failed: %d", ret); } - mcp_free(req); + mcp_free(tools_list_request); break; } case MCP_MSG_REQUEST_TOOLS_CALL: { - mcp_tools_call_request_t *req = (mcp_tools_call_request_t *)request.data; + mcp_tools_call_request_t *tools_call_request = + (mcp_tools_call_request_t *)request.data; - ret = handle_tools_call_request(req); + ret = handle_tools_call_request(tools_call_request); if (ret != 0) { LOG_ERR("Tools call request failed: %d", ret); } - mcp_free(req); + mcp_free(tools_call_request); break; } #endif case MCP_MSG_NOTIFICATION: { - mcp_client_notification_t *notif = + mcp_client_notification_t *notification = (mcp_client_notification_t *)request.data; - ret = handle_notification(notif); + ret = handle_notification(notification); if (ret != 0) { LOG_ERR("Notification failed: %d", ret); } - mcp_free(notif); + mcp_free(notification); break; } @@ -671,21 +717,34 @@ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) } } -static void mcp_message_worker(void *arg1, void *arg2, void *arg3) +static void mcp_response_worker(void *arg1, void *arg2, void *arg3) { - struct mcp_message_msg msg; + mcp_response_queue_msg_t response; int worker_id = POINTER_TO_INT(arg1); int ret; - LOG_INF("Message worker %d started", worker_id); + LOG_INF("Response worker %d started", worker_id); while (1) { - ret = k_msgq_get(&mcp_message_queue, &msg, K_FOREVER); + ret = k_msgq_get(&mcp_message_queue, &response, K_FOREVER); if (ret == 0) { - LOG_DBG("Processing message (worker %d)", worker_id); - /* TODO: Implement message processing */ + LOG_DBG("Processing response (worker %d)", worker_id); + /* TODO: When Server-sent Events and Authorization are added, implement the + * processing + */ + /* In phase 1, the response worker queue and workers are pretty much + * redundant by design + */ + /* TODO: Consider making response workers a configurable resource that can + * be bypassed + */ + ret = mcp_transport_queue_response(response.type, response.data); + if (ret != 0) { + LOG_ERR("Failed to queue response to transport: %d", ret); + mcp_free(response.data); + } } else { - LOG_ERR("Failed to get message: %d", ret); + LOG_ERR("Failed to get response: %d", ret); } } } @@ -751,7 +810,7 @@ int mcp_server_start(void) for (int i = 0; i < MCP_MESSAGE_WORKERS; i++) { tid = k_thread_create(&mcp_message_workers[i], mcp_message_worker_stacks[i], K_THREAD_STACK_SIZEOF(mcp_message_worker_stacks[i]), - mcp_message_worker, INT_TO_POINTER(i), NULL, NULL, + mcp_response_worker, INT_TO_POINTER(i), NULL, NULL, K_PRIO_COOP(MCP_WORKER_PRIORITY), 0, K_NO_WAIT); if (tid == NULL) { LOG_ERR("Failed to create message worker %d", i); @@ -770,9 +829,137 @@ int mcp_server_start(void) return 0; } -int mcp_queue_response(void) +int mcp_server_submit_app_message(const mcp_app_message_t *app_msg, uint32_t execution_token) { - /* TODO: Implement response queuing */ + int ret; + int execution_token_index; + uint32_t request_id; + uint32_t client_id; + mcp_response_queue_msg_t response; + + if (app_msg == NULL || app_msg->data == NULL) { + LOG_ERR("Invalid user message"); + return -EINVAL; + } + + if (execution_token == 0) { + LOG_ERR("Invalid execution token"); + return -EINVAL; + } + + ret = k_mutex_lock(&execution_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock execution registry: %d", ret); + return ret; + } + + execution_token_index = find_token_index(execution_token); + if (execution_token_index == -1) { + LOG_ERR("Execution token not found"); + k_mutex_unlock(&execution_registry.registry_mutex); + return -ENOENT; + } + + mcp_execution_context_t *execution_context = + &execution_registry.executions[execution_token_index]; + request_id = execution_context->request_id; + client_id = execution_context->client_id; + + if (app_msg->type == MCP_USR_TOOL_RESPONSE) { + execution_context->execution_state = MCP_EXEC_FINISHED; + } + + execution_context->last_message_timestamp = k_uptime_get(); + k_mutex_unlock(&execution_registry.registry_mutex); + + switch (app_msg->type) { + /* Result is passed as a complete JSON string that gets attached to the full JSON + * RPC response inside the Transport layer + */ +#ifdef CONFIG_MCP_TOOLS_CAPABILITY + case MCP_USR_TOOL_RESPONSE: { + mcp_tools_call_response_t *response_data = + (mcp_tools_call_response_t *)mcp_alloc(sizeof(mcp_tools_call_response_t)); + if (response_data == NULL) { + LOG_ERR("Failed to allocate memory for response"); + return -ENOMEM; + } + + response_data->request_id = request_id; + response_data->length = app_msg->length; + + if (app_msg->length > CONFIG_MCP_TOOL_RESULT_MAX_LEN) { + /* TODO: Truncate or fail? */ + LOG_WRN("Result truncated to max length"); + response_data->length = CONFIG_MCP_TOOL_RESULT_MAX_LEN; + response_data->result[CONFIG_MCP_TOOL_RESULT_MAX_LEN - 1] = '\0'; + } + + strncpy((char *)response_data->result, (char *)app_msg->data, + response_data->length); + + response.type = MCP_MSG_RESPONSE_TOOLS_CALL; + response.data = response_data; + break; + } +#endif + + default: + LOG_ERR("Unsupported application message type: %u", app_msg->type); + return -EINVAL; + } + + /* TODO: Make response workers configurable? If not used, call transport API directly? */ + ret = k_msgq_put(&mcp_message_queue, &response, K_NO_WAIT); + if (ret != 0) { + LOG_ERR("Failed to submit response to message queue: %d", ret); + mcp_free(response.data); + return ret; + } + + if (app_msg->type == MCP_USR_TOOL_RESPONSE) { + ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); + if (ret != 0) { + LOG_ERR("Failed to lock client registry mutex: %d. Client registry is " + "broken.", + ret); + goto skip_client_cleanup; + } + + int client_index = find_client_index(client_id); + + if (client_index == -1) { + LOG_ERR("Failed to find client index in the client registry. Client " + "registry is broken."); + k_mutex_unlock(&client_registry.registry_mutex); + goto skip_client_cleanup; + } + + int request_index = find_request_index(client_index, request_id); + + if (ret == -1) { + LOG_ERR("Failed to find request index in client's active requests. Client " + "registry is broken."); + k_mutex_unlock(&client_registry.registry_mutex); + goto skip_client_cleanup; + } + + client_registry.clients[client_index].active_requests[request_index] = 0; + client_registry.clients[client_index].active_request_count--; + + k_mutex_unlock(&client_registry.registry_mutex); + +skip_client_cleanup: + ret = destroy_execution_context(execution_token); + if (ret != 0) { + LOG_ERR("Failed to destroy execution context: %d. Execution registry is " + "broken. Message was submitted successfully.", + ret); + return ret; + } + /* TODO: Any way to wait until request worker is released? */ + } + return 0; } diff --git a/subsys/net/lib/mcp/mcp_transport.c b/subsys/net/lib/mcp/mcp_transport.c index f8721790a0dd8..c2bdbe554433d 100644 --- a/subsys/net/lib/mcp/mcp_transport.c +++ b/subsys/net/lib/mcp/mcp_transport.c @@ -11,20 +11,61 @@ #ifdef CONFIG_ZTEST int mcp_transport_queue_call_count; -mcp_response_queue_msg_t mcp_transport_last_queued_msg = {0}; +mcp_transport_queue_msg_t mcp_transport_last_queued_msg = {0}; #endif -int mcp_transport_queue_response(mcp_response_queue_msg_t *msg) +int mcp_transport_queue_response(mcp_queue_msg_type_t type, void *msg_data) { + /* queue msg to the correct requests queue */ + mcp_transport_queue_msg_t msg; + + msg.type = type; + + switch (type) { + case MCP_MSG_SYSTEM: + /* Handle system messages */ + break; + case MCP_MSG_NOTIFICATION: + /* Notification is not tied to a request_id, use client_id to send directly to + * client, when server to client notifications are implemented (e.g. list_changed), + * keep msg.data as NULL + */ + break; + case MCP_MSG_RESPONSE_INITIALIZE: + mcp_initialize_response_t *init_response = (mcp_initialize_response_t *)msg_data; + + msg.data = msg_data; + /* Locate correct k_msg_q based on init_response->request_id */ + /* Queue message to the transport layer */ + break; + case MCP_MSG_RESPONSE_TOOLS_LIST: + mcp_tools_list_response_t *tools_list_response = + (mcp_tools_list_response_t *)msg_data; + msg.data = msg_data; + /* Locate correct k_msg_q based on tools_list_response->request_id */ + /* Queue message to the transport layer */ + break; + case MCP_MSG_RESPONSE_TOOLS_CALL: + mcp_tools_call_response_t *tools_call_response = + (mcp_tools_call_response_t *)msg_data; + msg.data = msg_data; + /* Locate correct k_msg_q based on tools_list_response->request_id */ + /* Queue message to the transport layer */ + break; + default: + /* Unsupported message type */ + mcp_free(msg_data); + return -EINVAL; + } + #ifdef CONFIG_ZTEST /* Store call information for testing */ mcp_transport_queue_call_count++; - if (msg) { - mcp_transport_last_queued_msg.type = msg->type; - mcp_transport_last_queued_msg.data = msg->data; - } -#endif + mcp_transport_last_queued_msg.type = type; + mcp_transport_last_queued_msg.data = msg_data; - /* queue msg to the correct requests queue */ + /* TODO: Remove when transport queue is actually implemented! */ + mcp_free(msg_data); +#endif return 0; } diff --git a/subsys/net/lib/mcp/mcp_transport.h b/subsys/net/lib/mcp/mcp_transport.h index b361544589899..b96c8fc76d810 100644 --- a/subsys/net/lib/mcp/mcp_transport.h +++ b/subsys/net/lib/mcp/mcp_transport.h @@ -11,6 +11,6 @@ #include #include "mcp_common.h" -int mcp_transport_queue_response(mcp_response_queue_msg_t *msg); +int mcp_transport_queue_response(mcp_queue_msg_type_t type, void *msg_data); #endif /* ZEPHYR_SUBSYS_MCP_TRANSPORT_H_ */ diff --git a/tests/net/lib/mcp/src/main.c b/tests/net/lib/mcp/src/main.c index e01f2c81ff9b7..965b12b88422a 100644 --- a/tests/net/lib/mcp/src/main.c +++ b/tests/net/lib/mcp/src/main.c @@ -15,9 +15,11 @@ extern struct k_msgq mcp_request_queue; extern struct k_msgq mcp_message_queue; extern int mcp_transport_queue_call_count; -extern mcp_response_queue_msg_t mcp_transport_last_queued_msg; - +extern mcp_transport_queue_msg_t mcp_transport_last_queued_msg; +/* Tool execution test callbacks with different behaviors */ static int tool_execution_count; +static char last_execution_params[CONFIG_MCP_TOOL_INPUT_ARGS_MAX_LEN]; +static uint32_t last_execution_token; #ifdef CONFIG_ZTEST extern uint8_t mcp_server_get_client_count(void); @@ -63,37 +65,6 @@ extern uint8_t mcp_server_get_tool_count(void); #define MCP_MSG_INVALID_TYPE 0xFF -static int stub_tool_callback_1(const char *params, uint32_t execution_token) -{ - return 0; -} - -static int stub_tool_callback_2(const char *params, uint32_t execution_token) -{ - return 0; -} - -static int stub_tool_callback_3(const char *params, uint32_t execution_token) -{ - return 0; -} - -static void reset_transport_mock(void) -{ - mcp_transport_queue_call_count = 0; - memset(&mcp_transport_last_queued_msg, 0, sizeof(mcp_transport_last_queued_msg)); -} - -static int test_execution_tool_callback(const char *params, uint32_t execution_token) -{ - tool_execution_count++; - - printk("Tool execution callback executed! Count: %d, Token: %u, Arguments: %s\n", - tool_execution_count, execution_token, params ? params : "(null)"); - - return 0; -} - static void send_tools_call_request(uint32_t client_id, uint32_t request_id, const char *tool_name, const char *arguments) { @@ -216,6 +187,566 @@ static void initialize_client_fully(uint32_t client_id, uint32_t request_id) send_initialized_notification(client_id); } +static void reset_transport_mock(void) +{ + mcp_transport_queue_call_count = 0; + memset(&mcp_transport_last_queued_msg, 0, sizeof(mcp_transport_last_queued_msg)); +} +static int stub_tool_callback_1(const char *params, uint32_t execution_token) +{ + int ret; + mcp_app_message_t response; + char result_data[] = "{" + "\"content\": [" + "{" + "\"type\": \"text\"," + "\"text\": \"Hello world from callback 1. This tool processed the " + "request successfully.\"" + "}" + "]," + "\"isError\": false" + "}"; + + printk("Stub tool 1 executed - Token: %u, Args: %s\n", execution_token, + params ? params : "(null)"); + + response.type = MCP_USR_TOOL_RESPONSE; + response.data = result_data; + response.length = strlen(result_data); + + ret = mcp_server_submit_app_message(&response, execution_token); + if (ret != 0) { + printk("Failed to submit response from callback 1: %d\n", ret); + return ret; + } + + return 0; +} + +static int stub_tool_callback_2(const char *params, uint32_t execution_token) +{ + int ret; + mcp_app_message_t response; + char result_data[] = "{" + "\"content\": [" + "{" + "\"type\": \"text\"," + "\"text\": \"Hello world from callback 2. Tool execution completed.\"" + "}" + "]," + "\"isError\": false" + "}"; + + printk("Stub tool 2 executed - Token: %u, Args: %s\n", execution_token, + params ? params : "(null)"); + + response.type = MCP_USR_TOOL_RESPONSE; + response.data = result_data; + response.length = strlen(result_data); + + ret = mcp_server_submit_app_message(&response, execution_token); + if (ret != 0) { + printk("Failed to submit response from callback 2: %d\n", ret); + return ret; + } + + return 0; +} + +static int stub_tool_callback_3(const char *params, uint32_t execution_token) +{ + int ret; + mcp_app_message_t response; + char result_data[] = + "{" + "\"content\": [" + "{" + "\"type\": \"text\"," + "\"text\": \"Hello world from callback 3. Registry tool execution successful.\"" + "}" + "]," + "\"isError\": false" + "}"; + + printk("Stub tool 3 executed - Token: %u, Args: %s\n", execution_token, + params ? params : "(null)"); + + response.type = MCP_USR_TOOL_RESPONSE; + response.data = result_data; + response.length = strlen(result_data); + + ret = mcp_server_submit_app_message(&response, execution_token); + if (ret != 0) { + printk("Failed to submit response from callback 3: %d\n", ret); + return ret; + } + + return 0; +} + +static int test_tool_success_callback(const char *params, uint32_t execution_token) +{ + int ret; + mcp_app_message_t response; + char result_data[512]; + char text_content[256]; + + tool_execution_count++; + last_execution_token = execution_token; + + if (params) { + strncpy(last_execution_params, params, sizeof(last_execution_params) - 1); + last_execution_params[sizeof(last_execution_params) - 1] = '\0'; + } + + snprintf(text_content, sizeof(text_content), + "Success tool executed successfully. Execution count: %d. Input parameters: %s", + tool_execution_count, params ? params : "none"); + + snprintf(result_data, sizeof(result_data), + "{" + "\"content\": [" + "{" + "\"type\": \"text\"," + "\"text\": \"%s\"" + "}" + "]," + "\"isError\": false" + "}", + text_content); + + printk("SUCCESS tool executed! Count: %d, Token: %u, Args: %s\n", tool_execution_count, + execution_token, params ? params : "(null)"); + + response.type = MCP_USR_TOOL_RESPONSE; + response.data = result_data; + response.length = strlen(result_data); + + ret = mcp_server_submit_app_message(&response, execution_token); + if (ret != 0) { + printk("Failed to submit response from success callback: %d\n", ret); + return ret; + } + + return 0; +} + +static int test_tool_error_callback(const char *params, uint32_t execution_token) +{ + int ret; + mcp_app_message_t response; + char result_data[] = "{" + "\"content\": [" + "{" + "\"type\": \"text\"," + "\"text\": \"Error: This tool intentionally failed to test error " + "handling. The operation could not be completed.\"" + "}" + "]," + "\"isError\": true" + "}"; + + tool_execution_count++; + + printk("ERROR tool executed! Count: %d, Token: %u, Args: %s (submitting error response)\n", + tool_execution_count, execution_token, params ? params : "(null)"); + + response.type = MCP_USR_TOOL_RESPONSE; + response.data = result_data; + response.length = strlen(result_data); + + ret = mcp_server_submit_app_message(&response, execution_token); + if (ret != 0) { + printk("Failed to submit response from error callback: %d\n", ret); + return ret; + } + + return 0; +} + +static int test_tool_slow_callback(const char *params, uint32_t execution_token) +{ + int ret; + mcp_app_message_t response; + char result_data[] = "{" + "\"content\": [" + "{" + "\"type\": \"text\"," + "\"text\": \"Slow operation completed successfully. The task took " + "100ms to simulate a long-running operation.\"" + "}" + "]," + "\"isError\": false" + "}"; + + tool_execution_count++; + + printk("SLOW tool starting execution! Token: %u\n", execution_token); + k_msleep(100); /* Simulate slow operation */ + printk("SLOW tool completed execution! Token: %u\n", execution_token); + + response.type = MCP_USR_TOOL_RESPONSE; + response.data = result_data; + response.length = strlen(result_data); + + ret = mcp_server_submit_app_message(&response, execution_token); + if (ret != 0) { + printk("Failed to submit response from slow callback: %d\n", ret); + return ret; + } + + return 0; +} +/* Reset tool execution tracking */ +static void reset_tool_execution_tracking(void) +{ + tool_execution_count = 0; + last_execution_token = 0; + memset(last_execution_params, 0, sizeof(last_execution_params)); +} + +/* Register test tools for comprehensive testing */ +static void register_test_tools(void) +{ + int ret; + + mcp_tool_record_t success_tool = { + .metadata = { + .name = "test_success_tool", + .input_schema = "{\"type\":\"object\",\"properties\":{\"message\":{" + "\"type\":\"string\"}}}", +#ifdef CONFIG_MCP_TOOL_DESC + .description = "Tool that always succeeds", +#endif +#ifdef CONFIG_MCP_TOOL_TITLE + .title = "Success Test Tool", +#endif +#ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA + .output_schema = "{\"type\":\"object\",\"properties\":{\"result\":{" + "\"type\":\"string\"}}}", +#endif + }, + .callback = test_tool_success_callback + }; + + mcp_tool_record_t error_tool = { + .metadata = { + .name = "test_error_tool", + .input_schema = "{\"type\":\"object\"}", +#ifdef CONFIG_MCP_TOOL_DESC + .description = "Tool that always returns error", +#endif + }, + .callback = test_tool_error_callback + }; + + mcp_tool_record_t slow_tool = { + .metadata = { + .name = "test_slow_tool", + .input_schema = "{\"type\":\"object\"}", +#ifdef CONFIG_MCP_TOOL_DESC + .description = "Tool that takes time to execute", +#endif + }, + .callback = test_tool_slow_callback + }; + + ret = mcp_server_add_tool(&success_tool); + zassert_equal(ret, 0, "Success tool should register"); + + ret = mcp_server_add_tool(&error_tool); + zassert_equal(ret, 0, "Error tool should register"); + + ret = mcp_server_add_tool(&slow_tool); + zassert_equal(ret, 0, "Slow tool should register"); +} + +/* Clean up test tools */ +static void cleanup_test_tools(void) +{ + mcp_server_remove_tool("test_success_tool"); + mcp_server_remove_tool("test_error_tool"); + mcp_server_remove_tool("test_slow_tool"); +} + +ZTEST(mcp_server_tests, test_tools_call_comprehensive) +{ + uint8_t initial_tool_count = mcp_server_get_tool_count(); + int initial_transport_count; + + reset_tool_execution_tracking(); + reset_transport_mock(); + + /* Register test tools */ + register_test_tools(); + zassert_equal(mcp_server_get_tool_count(), initial_tool_count + 3, + "All test tools should be registered"); + + /* Initialize client */ + initialize_client_fully(CLIENT_ID_EDGE_CASE_TEST, REQ_ID_EDGE_CASE_INITIALIZE); + + /* Test 1: Successful tool execution with parameters */ + printk("=== Test 1: Successful tool execution ===\n"); + reset_transport_mock(); + send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 3001, "test_success_tool", + "{\"message\":\"hello world\"}"); + + /* Verify tool executed AND response was submitted to transport */ + zassert_equal(tool_execution_count, 1, "Success tool should execute once"); + zassert_equal(mcp_transport_queue_call_count, 1, + "Tool response should be submitted to transport"); + zassert_equal(mcp_transport_last_queued_msg.type, MCP_MSG_RESPONSE_TOOLS_CALL, + "Transport should receive tools/call response"); + + /* Verify response content contains expected data */ + mcp_tools_call_response_t *response = + (mcp_tools_call_response_t *)mcp_transport_last_queued_msg.data; + zassert_not_null(response, "Response data should not be NULL"); + zassert_equal(response->request_id, 3001, "Response should have correct request ID"); + zassert_true(strstr(response->result, "Success tool executed successfully") != NULL, + "Response should contain success message"); + + /* Test 2: Tool execution with empty parameters */ + printk("=== Test 2: Tool execution with empty parameters ===\n"); + initial_transport_count = mcp_transport_queue_call_count; + reset_transport_mock(); + send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 3002, "test_success_tool", ""); + + zassert_equal(tool_execution_count, 2, "Tool should execute twice total"); + zassert_equal(mcp_transport_queue_call_count, 1, + "Second tool response should be submitted"); + zassert_equal(mcp_transport_last_queued_msg.type, MCP_MSG_RESPONSE_TOOLS_CALL, + "Transport should receive second tools/call response"); + + /* Test 3: Tool execution with NULL parameters */ + printk("=== Test 3: Tool execution with NULL parameters ===\n"); + reset_transport_mock(); + send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 3003, "test_success_tool", NULL); + + zassert_equal(tool_execution_count, 3, "Tool should execute three times total"); + zassert_equal(mcp_transport_queue_call_count, 1, "Third tool response should be submitted"); + + /* Test 4: Tool that returns error */ + printk("=== Test 4: Tool that returns error ===\n"); + reset_transport_mock(); + send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 3004, "test_error_tool", + "{\"test\":\"data\"}"); + + /* Error tool should still execute and submit response */ + zassert_equal(tool_execution_count, 4, "Error tool should still execute"); + zassert_equal(mcp_transport_queue_call_count, 1, "Error tool response should be submitted"); + zassert_equal(mcp_transport_last_queued_msg.type, MCP_MSG_RESPONSE_TOOLS_CALL, + "Transport should receive error tools/call response"); + + /* Verify error response content */ + response = (mcp_tools_call_response_t *)mcp_transport_last_queued_msg.data; + zassert_not_null(response, "Error response data should not be NULL"); + zassert_equal(response->request_id, 3004, "Error response should have correct request ID"); + zassert_true(strstr(response->result, "isError\": true") != NULL, + "Error response should indicate error"); + + /* Test 5: Non-existent tool */ + printk("=== Test 5: Non-existent tool ===\n"); + int execution_count_before_nonexistent = tool_execution_count; + + reset_transport_mock(); + send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 3005, "non_existent_tool", "{}"); + + zassert_equal(tool_execution_count, execution_count_before_nonexistent, + "Non-existent tool should not execute"); + zassert_equal(mcp_transport_queue_call_count, 0, + "No response should be submitted for non-existent tool"); + + /* Test 6: Tool execution from unregistered client */ + printk("=== Test 6: Unregistered client ===\n"); + int execution_count_before_unregistered = tool_execution_count; + + reset_transport_mock(); + send_tools_call_request(CLIENT_ID_UNREGISTERED, 3006, "test_success_tool", "{}"); + + zassert_equal(tool_execution_count, execution_count_before_unregistered, + "Unregistered client should not execute tools"); + zassert_equal(mcp_transport_queue_call_count, 0, + "No response should be submitted for unregistered client"); + + /* Test 7: Tool execution from non-initialized client */ + printk("=== Test 7: Non-initialized client ===\n"); + /* Register a new client but don't send initialized notification */ + send_initialize_request(CLIENT_ID_INITIALIZE_TEST, 3007); + + int execution_count_before_uninitialized = tool_execution_count; + + reset_transport_mock(); + send_tools_call_request(CLIENT_ID_INITIALIZE_TEST, 3008, "test_success_tool", "{}"); + + zassert_equal(tool_execution_count, execution_count_before_uninitialized, + "Non-initialized client should not execute tools"); + zassert_equal(mcp_transport_queue_call_count, 0, + "No response should be submitted for non-initialized client"); + + /* Test 8: Multiple tool executions */ + printk("=== Test 8: Multiple tool executions ===\n"); + int execution_count_before_multiple = tool_execution_count; + + send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 3009, "test_success_tool", + "{\"test\":\"1\"}"); + send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 3010, "test_success_tool", + "{\"test\":\"2\"}"); + send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 3011, "test_success_tool", + "{\"test\":\"3\"}"); + + zassert_equal(tool_execution_count, execution_count_before_multiple + 3, + "Multiple tool executions should work"); + + /* Test 9: Long parameter string */ + printk("=== Test 9: Long parameter string ===\n"); + static char long_params[CONFIG_MCP_TOOL_INPUT_ARGS_MAX_LEN]; + + memset(long_params, 'x', sizeof(long_params) - 1); + long_params[sizeof(long_params) - 1] = '\0'; + + int execution_count_before_long = tool_execution_count; + + reset_transport_mock(); + send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 3012, "test_success_tool", long_params); + k_msleep(200); + + zassert_equal(tool_execution_count, execution_count_before_long + 1, + "Long parameter string should work"); + zassert_equal(mcp_transport_queue_call_count, 1, + "Long parameter tool response should be submitted"); + + /* Test 10: Slow tool execution */ + printk("=== Test 10: Slow tool execution ===\n"); + int execution_count_before_slow = tool_execution_count; + + reset_transport_mock(); + send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 3013, "test_slow_tool", "{}"); + k_msleep(300); /* Wait longer for slow tool */ + + zassert_equal(tool_execution_count, execution_count_before_slow + 1, + "Slow tool should complete execution"); + zassert_equal(mcp_transport_queue_call_count, 1, "Slow tool response should be submitted"); + zassert_equal(mcp_transport_last_queued_msg.type, MCP_MSG_RESPONSE_TOOLS_CALL, + "Transport should receive slow tool response"); + + /* Clean up */ + cleanup_test_tools(); + send_client_shutdown(CLIENT_ID_EDGE_CASE_TEST); + send_client_shutdown(CLIENT_ID_INITIALIZE_TEST); + + zassert_equal(mcp_server_get_tool_count(), initial_tool_count, + "Tool count should return to initial value"); + + printk("=== Comprehensive tools/call testing completed ===\n"); +} + +ZTEST(mcp_server_tests, test_tools_call_concurrent_clients) +{ + uint8_t initial_tool_count = mcp_server_get_tool_count(); + + reset_tool_execution_tracking(); + reset_transport_mock(); + + /* Register a test tool */ + mcp_tool_record_t concurrent_tool = { + .metadata = { + .name = "concurrent_test_tool", + .input_schema = "{\"type\":\"object\"}", +#ifdef CONFIG_MCP_TOOL_DESC + .description = "Tool for concurrent testing", +#endif + }, + .callback = test_tool_success_callback}; + + int ret = mcp_server_add_tool(&concurrent_tool); + + zassert_equal(ret, 0, "Concurrent test tool should register"); + + /* Initialize multiple clients */ + initialize_client_fully(CLIENT_ID_MULTI_CLIENT_1, REQ_ID_MULTI_CLIENT_1_INIT); + initialize_client_fully(CLIENT_ID_MULTI_CLIENT_2, REQ_ID_MULTI_CLIENT_2_INIT); + + printk("=== Testing concurrent tool execution from multiple clients ===\n"); + + /* Execute tool from both clients */ + send_tools_call_request(CLIENT_ID_MULTI_CLIENT_1, 4001, "concurrent_test_tool", + "{\"client\":\"1\"}"); + k_msleep(200); + send_tools_call_request(CLIENT_ID_MULTI_CLIENT_2, 4002, "concurrent_test_tool", + "{\"client\":\"2\"}"); + k_msleep(200); + + /* Both executions should succeed */ + zassert_equal(tool_execution_count, 2, "Tool should execute from both clients"); + + /* Clean up */ + mcp_server_remove_tool("concurrent_test_tool"); + send_client_shutdown(CLIENT_ID_MULTI_CLIENT_1); + send_client_shutdown(CLIENT_ID_MULTI_CLIENT_2); + + zassert_equal(mcp_server_get_tool_count(), initial_tool_count, + "Tool count should return to initial value"); + + printk("=== Concurrent client testing completed ===\n"); +} + +ZTEST(mcp_server_tests, test_tools_call_edge_cases) +{ + uint8_t initial_tool_count = mcp_server_get_tool_count(); + + reset_tool_execution_tracking(); + reset_transport_mock(); + + /* Register a test tool */ + mcp_tool_record_t edge_case_tool = { + .metadata = { + .name = "edge_case_tool", + .input_schema = "{\"type\":\"object\"}", + }, + .callback = test_tool_success_callback + }; + + int ret = mcp_server_add_tool(&edge_case_tool); + + zassert_equal(ret, 0, "Edge case tool should register"); + + initialize_client_fully(CLIENT_ID_EDGE_CASE_TEST, REQ_ID_EDGE_CASE_INITIALIZE); + + printk("=== Testing edge cases for tools/call ===\n"); + + /* Test: Very long tool name (should not match) */ + char long_tool_name[CONFIG_MCP_TOOL_NAME_MAX_LEN + 10]; + + memset(long_tool_name, 'a', sizeof(long_tool_name) - 1); + + long_tool_name[sizeof(long_tool_name) - 1] = '\0'; + + send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 5001, long_tool_name, "{}"); + k_msleep(200); + zassert_equal(tool_execution_count, 0, "Tool with long name should not execute"); + + /* Test: Empty tool name */ + send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 5002, "", "{}"); + k_msleep(200); + zassert_equal(tool_execution_count, 0, "Tool with empty name should not execute"); + + /* Test: Special characters in parameters */ + send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 5003, "edge_case_tool", + "{\"special\":\"\\\"quotes\\\"\"}"); + k_msleep(200); + zassert_equal(tool_execution_count, 1, "Tool with special characters should execute"); + + /* Clean up */ + mcp_server_remove_tool("edge_case_tool"); + send_client_shutdown(CLIENT_ID_EDGE_CASE_TEST); + + zassert_equal(mcp_server_get_tool_count(), initial_tool_count, + "Tool count should return to initial value"); + + printk("=== Edge case testing completed ===\n"); +} + ZTEST(mcp_server_tests, test_memory_allocation) { void *ptr1 = mcp_alloc(100); @@ -232,6 +763,7 @@ ZTEST(mcp_server_tests, test_memory_allocation) mcp_free(ptr2); mcp_free(NULL); } + ZTEST(mcp_server_tests, test_tools_list_response) { int ret; @@ -389,6 +921,7 @@ ZTEST(mcp_server_tests, test_initialize_request) send_client_shutdown(CLIENT_ID_INITIALIZE_TEST); } + ZTEST(mcp_server_tests, test_tool_registration_valid) { int ret; @@ -703,19 +1236,6 @@ ZTEST(mcp_server_tests, test_server_double_init) zassert_equal(ret, 0, "Second server init should succeed or handle gracefully"); } -ZTEST(mcp_server_tests, test_message_worker) -{ - int ret; - struct mcp_message_msg test_msg; - - memset(&test_msg, 0, sizeof(test_msg)); - - ret = k_msgq_put(&mcp_message_queue, &test_msg, K_NO_WAIT); - zassert_equal(ret, 0, "Message queueing should succeed"); - - k_msleep(100); -} - ZTEST(mcp_server_tests, test_null_data_request) { int ret; @@ -746,21 +1266,20 @@ ZTEST(mcp_server_tests, test_tools_call) mcp_tool_record_t execution_tool = { .metadata = { - .name = "execution_test_tool", - .input_schema = "{\"type\":\"object\",\"properties\":" - "{\"param1\":{\"type\":\"string\"}}}", + .name = "execution_test_tool", + .input_schema = "{\"type\":\"object\",\"properties\":" + "{\"param1\":{\"type\":\"string\"}}}", #ifdef CONFIG_MCP_TOOL_DESC - .description = "Tool for testing execution", + .description = "Tool for testing execution", #endif #ifdef CONFIG_MCP_TOOL_TITLE - .title = "Execution Test Tool", + .title = "Execution Test Tool", #endif #ifdef CONFIG_MCP_TOOL_OUTPUT_SCHEMA - .output_schema = "{\"type\":\"string\"}", + .output_schema = "{\"type\":\"string\"}", #endif - }, - .callback = test_execution_tool_callback - }; + }, + .callback = test_tool_success_callback}; ret = mcp_server_add_tool(&execution_tool); zassert_equal(ret, 0, "Execution test tool should register successfully"); @@ -801,6 +1320,100 @@ ZTEST(mcp_server_tests, test_tools_call) printk("Tool execution test completed successfully\n"); } +ZTEST(mcp_server_tests, test_invalid_execution_tokens) +{ + int ret; + mcp_app_message_t app_msg; + char response_data[] = "{" + "\"content\": [" + "{" + "\"type\": \"text\"," + "\"text\": \"This should not be accepted\"" + "}" + "]," + "\"isError\": false" + "}"; + + app_msg.type = MCP_USR_TOOL_RESPONSE; + app_msg.data = response_data; + app_msg.length = strlen(response_data); + + printk("=== Testing invalid execution tokens ===\n"); + + /* Test 1: Zero execution token (invalid) */ + printk("=== Test 1: Zero execution token ===\n"); + ret = mcp_server_submit_app_message(&app_msg, 0); + zassert_equal(ret, -EINVAL, "Zero execution token should be rejected with -EINVAL"); + + /* Test 2: Non-existent execution token */ + printk("=== Test 2: Non-existent execution token ===\n"); + + uint32_t fake_token = 99999; + + ret = mcp_server_submit_app_message(&app_msg, fake_token); + zassert_equal(ret, -ENOENT, "Non-existent execution token should be rejected with -ENOENT"); + + /* Test 3: Create a valid token, use it, then try to use it again */ + printk("=== Test 3: Reusing completed execution token ===\n"); + + uint8_t initial_tool_count = mcp_server_get_tool_count(); + + /* Register a test tool for this test */ + mcp_tool_record_t token_test_tool = { + .metadata = { + .name = "token_test_tool", + .input_schema = "{\"type\":\"object\"}", +#ifdef CONFIG_MCP_TOOL_DESC + .description = "Tool for testing execution tokens", +#endif + }, + .callback = test_tool_success_callback}; + + ret = mcp_server_add_tool(&token_test_tool); + zassert_equal(ret, 0, "Test tool should register successfully"); + + /* Initialize a client and execute a tool to get a valid token */ + initialize_client_fully(CLIENT_ID_EDGE_CASE_TEST, REQ_ID_EDGE_CASE_INITIALIZE); + + /* Reset execution tracking */ + reset_tool_execution_tracking(); + + /* Execute the tool - this will create and then destroy an execution token */ + send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 4500, "token_test_tool", "{}"); + k_msleep(200); /* Wait for tool execution to complete */ + + zassert_equal(tool_execution_count, 1, "Tool should have executed once"); + uint32_t used_token = last_execution_token; + + zassert_not_equal(used_token, 0, "Should have captured the execution token"); + + /* Now try to use the same token again - it should be rejected */ + printk("=== Test 3a: Attempting to reuse token %u ===\n", used_token); + ret = mcp_server_submit_app_message(&app_msg, used_token); + zassert_equal(ret, -ENOENT, "Completed execution token should be rejected with -ENOENT"); + + /* Test 4: NULL app_msg */ + printk("=== Test 4: NULL app_msg ===\n"); + ret = mcp_server_submit_app_message(NULL, 1234); + zassert_equal(ret, -EINVAL, "NULL app_msg should be rejected with -EINVAL"); + + /* Test 5: app_msg with NULL data */ + printk("=== Test 5: app_msg with NULL data ===\n"); + mcp_app_message_t null_data_msg = { + .type = MCP_USR_TOOL_RESPONSE, .data = NULL, .length = 10}; + ret = mcp_server_submit_app_message(&null_data_msg, 1234); + zassert_equal(ret, -EINVAL, "app_msg with NULL data should be rejected with -EINVAL"); + + /* Clean up */ + mcp_server_remove_tool("token_test_tool"); + send_client_shutdown(CLIENT_ID_EDGE_CASE_TEST); + + zassert_equal(mcp_server_get_tool_count(), initial_tool_count, + "Tool count should return to initial value"); + + printk("=== Invalid execution token testing completed ===\n"); +} + static void *mcp_server_tests_setup(void) { int ret; From b9544fc202e361a00f305d7ec2ee522a89131259 Mon Sep 17 00:00:00 2001 From: David Piskula Date: Mon, 24 Nov 2025 15:42:03 +0100 Subject: [PATCH 09/12] networking: error responses Added server error responses Signed-off-by: David Piskula --- subsys/net/lib/mcp/mcp_common.h | 11 ++++ subsys/net/lib/mcp/mcp_server.c | 73 ++++++++++++++++++++++++- subsys/net/lib/mcp/mcp_transport.c | 28 +++++++++- tests/net/lib/mcp/src/main.c | 86 ++++++++++++++++-------------- 4 files changed, 153 insertions(+), 45 deletions(-) diff --git a/subsys/net/lib/mcp/mcp_common.h b/subsys/net/lib/mcp/mcp_common.h index 74eb5421852df..9ccd2a40d7d47 100644 --- a/subsys/net/lib/mcp/mcp_common.h +++ b/subsys/net/lib/mcp/mcp_common.h @@ -27,6 +27,11 @@ typedef enum { #ifdef CONFIG_MCP_TOOLS_CAPABILITY MCP_MSG_RESPONSE_TOOLS_LIST, MCP_MSG_RESPONSE_TOOLS_CALL, +#endif + MCP_MSG_ERROR_INITIALIZE, +#ifdef CONFIG_MCP_TOOLS_CAPABILITY + MCP_MSG_ERROR_TOOLS_LIST, + MCP_MSG_ERROR_TOOLS_CALL, #endif MCP_MSG_NOTIFICATION, } mcp_queue_msg_type_t; @@ -58,6 +63,12 @@ typedef struct mcp_system_msg { uint32_t client_id; } mcp_system_msg_t; +typedef struct mcp_error_response { + uint32_t request_id; + int32_t error_code; + char error_message[128]; +} mcp_error_response_t; + typedef struct mcp_initialize_request { uint32_t request_id; uint32_t client_id; diff --git a/subsys/net/lib/mcp/mcp_server.c b/subsys/net/lib/mcp/mcp_server.c index 7cfa7bbe075e4..53ac1f6e71894 100644 --- a/subsys/net/lib/mcp/mcp_server.c +++ b/subsys/net/lib/mcp/mcp_server.c @@ -162,6 +162,34 @@ static int register_new_client(uint32_t client_id) return slot_index; } +static int send_error_response(uint32_t request_id, mcp_queue_msg_type_t error_type, + int32_t error_code, const char *error_message) +{ + mcp_error_response_t *error_response; + int ret; + + error_response = (mcp_error_response_t *)mcp_alloc(sizeof(mcp_error_response_t)); + if (!error_response) { + LOG_ERR("Failed to allocate error response"); + return -ENOMEM; + } + + error_response->request_id = request_id; + error_response->error_code = error_code; + strncpy(error_response->error_message, error_message, + sizeof(error_response->error_message) - 1); + error_response->error_message[sizeof(error_response->error_message) - 1] = '\0'; + + ret = mcp_transport_queue_response(error_type, error_response); + if (ret != 0) { + LOG_ERR("Failed to queue error response: %d", ret); + mcp_free(error_response); + return ret; + } + + return 0; +} + #ifdef CONFIG_MCP_TOOLS_CAPABILITY static uint32_t generate_execution_token(uint32_t request_id) { @@ -626,7 +654,6 @@ static int handle_notification(mcp_client_notification_t *notification) } /* Worker threads */ - static void mcp_request_worker(void *arg1, void *arg2, void *arg3) { mcp_request_queue_msg_t request; @@ -666,6 +693,11 @@ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) ret = handle_initialize_request(init_request); if (ret != 0) { LOG_ERR("Initialize request failed: %d", ret); + /* Send error response according to JSON-RPC spec */ + send_error_response(init_request->request_id, + MCP_MSG_ERROR_INITIALIZE, + -32603, /* Internal error */ + "Server initialization failed"); } mcp_free(init_request); break; @@ -679,6 +711,24 @@ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) ret = handle_tools_list_request(tools_list_request); if (ret != 0) { LOG_ERR("Tools list request failed: %d", ret); + const char *error_msg; + int32_t error_code; + + if (ret == -ENOENT) { + error_code = -32602; /* Invalid params */ + error_msg = "Client not found or not initialized"; + } else if (ret == -EPERM) { + error_code = -32602; /* Invalid params */ + error_msg = + "Client not in correct state for tools requests"; + } else { + error_code = -32603; /* Internal error */ + error_msg = "Internal server error processing tools list"; + } + + send_error_response(tools_list_request->request_id, + MCP_MSG_ERROR_TOOLS_LIST, error_code, + error_msg); } mcp_free(tools_list_request); break; @@ -691,12 +741,31 @@ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) ret = handle_tools_call_request(tools_call_request); if (ret != 0) { LOG_ERR("Tools call request failed: %d", ret); + const char *error_msg; + int32_t error_code; + + if (ret == -ENOENT) { + error_code = -32601; /* Method not found */ + error_msg = "Tool not found"; + } else if (ret == -EPERM) { + error_code = -32602; /* Invalid params */ + error_msg = "Client not authorized for tool execution"; + } else if (ret == -ENOSPC) { + error_code = -32603; /* Internal error */ + error_msg = "No available execution slots"; + } else { + error_code = -32603; /* Internal error */ + error_msg = "Internal server error processing tool call"; + } + + send_error_response(tools_call_request->request_id, + MCP_MSG_ERROR_TOOLS_CALL, error_code, + error_msg); } mcp_free(tools_call_request); break; } #endif - case MCP_MSG_NOTIFICATION: { mcp_client_notification_t *notification = (mcp_client_notification_t *)request.data; diff --git a/subsys/net/lib/mcp/mcp_transport.c b/subsys/net/lib/mcp/mcp_transport.c index c2bdbe554433d..fc430c8a49ea9 100644 --- a/subsys/net/lib/mcp/mcp_transport.c +++ b/subsys/net/lib/mcp/mcp_transport.c @@ -16,7 +16,6 @@ mcp_transport_queue_msg_t mcp_transport_last_queued_msg = {0}; int mcp_transport_queue_response(mcp_queue_msg_type_t type, void *msg_data) { - /* queue msg to the correct requests queue */ mcp_transport_queue_msg_t msg; msg.type = type; @@ -38,20 +37,45 @@ int mcp_transport_queue_response(mcp_queue_msg_type_t type, void *msg_data) /* Locate correct k_msg_q based on init_response->request_id */ /* Queue message to the transport layer */ break; + case MCP_MSG_ERROR_INITIALIZE: + mcp_error_response_t *init_error = (mcp_error_response_t *)msg_data; + + msg.data = msg_data; + /* Create JSON-RPC error response for initialize */ + /* Queue error message to the transport layer */ + break; +#ifdef CONFIG_MCP_TOOLS_CAPABILITY case MCP_MSG_RESPONSE_TOOLS_LIST: mcp_tools_list_response_t *tools_list_response = (mcp_tools_list_response_t *)msg_data; + msg.data = msg_data; /* Locate correct k_msg_q based on tools_list_response->request_id */ /* Queue message to the transport layer */ break; + case MCP_MSG_ERROR_TOOLS_LIST: + mcp_error_response_t *tools_list_error = (mcp_error_response_t *)msg_data; + + msg.data = msg_data; + /* Create JSON-RPC error response for tools/list */ + /* Queue error message to the transport layer */ + break; case MCP_MSG_RESPONSE_TOOLS_CALL: mcp_tools_call_response_t *tools_call_response = (mcp_tools_call_response_t *)msg_data; + msg.data = msg_data; - /* Locate correct k_msg_q based on tools_list_response->request_id */ + /* Locate correct k_msg_q based on tools_call_response->request_id */ /* Queue message to the transport layer */ break; + case MCP_MSG_ERROR_TOOLS_CALL: + mcp_error_response_t *tools_call_error = (mcp_error_response_t *)msg_data; + + msg.data = msg_data; + /* Create JSON-RPC error response for tools/call */ + /* Queue error message to the transport layer */ + break; +#endif default: /* Unsupported message type */ mcp_free(msg_data); diff --git a/tests/net/lib/mcp/src/main.c b/tests/net/lib/mcp/src/main.c index 965b12b88422a..81fd0182cbd90 100644 --- a/tests/net/lib/mcp/src/main.c +++ b/tests/net/lib/mcp/src/main.c @@ -94,7 +94,7 @@ static void send_tools_call_request(uint32_t client_id, uint32_t request_id, con ret = k_msgq_put(&mcp_request_queue, &msg, K_NO_WAIT); zassert_equal(ret, 0, "Tools call request queueing failed"); - k_msleep(100); + k_msleep(200); } static void send_initialize_request(uint32_t client_id, uint32_t request_id) @@ -545,44 +545,54 @@ ZTEST(mcp_server_tests, test_tools_call_comprehensive) zassert_true(strstr(response->result, "isError\": true") != NULL, "Error response should indicate error"); - /* Test 5: Non-existent tool */ + /* Test 5: Non-existent tool - now expects error response */ printk("=== Test 5: Non-existent tool ===\n"); int execution_count_before_nonexistent = tool_execution_count; - reset_transport_mock(); send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 3005, "non_existent_tool", "{}"); zassert_equal(tool_execution_count, execution_count_before_nonexistent, "Non-existent tool should not execute"); - zassert_equal(mcp_transport_queue_call_count, 0, - "No response should be submitted for non-existent tool"); - - /* Test 6: Tool execution from unregistered client */ + zassert_equal(mcp_transport_queue_call_count, 1, + "Error response should be submitted for non-existent tool"); + zassert_equal(mcp_transport_last_queued_msg.type, MCP_MSG_ERROR_TOOLS_CALL, + "Transport should receive tools/call error response"); + + /* Verify error response */ + mcp_error_response_t *error_response = + (mcp_error_response_t *)mcp_transport_last_queued_msg.data; + zassert_not_null(error_response, "Error response data should not be NULL"); + zassert_equal(error_response->request_id, 3005, + "Error response should have correct request ID"); + + /* Test 6: Tool execution from unregistered client - now expects error response */ printk("=== Test 6: Unregistered client ===\n"); int execution_count_before_unregistered = tool_execution_count; - reset_transport_mock(); send_tools_call_request(CLIENT_ID_UNREGISTERED, 3006, "test_success_tool", "{}"); zassert_equal(tool_execution_count, execution_count_before_unregistered, "Unregistered client should not execute tools"); - zassert_equal(mcp_transport_queue_call_count, 0, - "No response should be submitted for unregistered client"); + zassert_equal(mcp_transport_queue_call_count, 1, + "Error response should be submitted for unregistered client"); + zassert_equal(mcp_transport_last_queued_msg.type, MCP_MSG_ERROR_TOOLS_CALL, + "Transport should receive tools/call error response"); - /* Test 7: Tool execution from non-initialized client */ + /* Test 7: Tool execution from non-initialized client - now expects error response */ printk("=== Test 7: Non-initialized client ===\n"); /* Register a new client but don't send initialized notification */ send_initialize_request(CLIENT_ID_INITIALIZE_TEST, 3007); int execution_count_before_uninitialized = tool_execution_count; - reset_transport_mock(); send_tools_call_request(CLIENT_ID_INITIALIZE_TEST, 3008, "test_success_tool", "{}"); zassert_equal(tool_execution_count, execution_count_before_uninitialized, "Non-initialized client should not execute tools"); - zassert_equal(mcp_transport_queue_call_count, 0, - "No response should be submitted for non-initialized client"); + zassert_equal(mcp_transport_queue_call_count, 1, + "Error response should be submitted for non-initialized client"); + zassert_equal(mcp_transport_last_queued_msg.type, MCP_MSG_ERROR_TOOLS_CALL, + "Transport should receive tools/call error response"); /* Test 8: Multiple tool executions */ printk("=== Test 8: Multiple tool executions ===\n"); @@ -601,15 +611,12 @@ ZTEST(mcp_server_tests, test_tools_call_comprehensive) /* Test 9: Long parameter string */ printk("=== Test 9: Long parameter string ===\n"); static char long_params[CONFIG_MCP_TOOL_INPUT_ARGS_MAX_LEN]; - memset(long_params, 'x', sizeof(long_params) - 1); long_params[sizeof(long_params) - 1] = '\0'; int execution_count_before_long = tool_execution_count; - reset_transport_mock(); send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 3012, "test_success_tool", long_params); - k_msleep(200); zassert_equal(tool_execution_count, execution_count_before_long + 1, "Long parameter string should work"); @@ -619,10 +626,8 @@ ZTEST(mcp_server_tests, test_tools_call_comprehensive) /* Test 10: Slow tool execution */ printk("=== Test 10: Slow tool execution ===\n"); int execution_count_before_slow = tool_execution_count; - reset_transport_mock(); send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 3013, "test_slow_tool", "{}"); - k_msleep(300); /* Wait longer for slow tool */ zassert_equal(tool_execution_count, execution_count_before_slow + 1, "Slow tool should complete execution"); @@ -672,10 +677,8 @@ ZTEST(mcp_server_tests, test_tools_call_concurrent_clients) /* Execute tool from both clients */ send_tools_call_request(CLIENT_ID_MULTI_CLIENT_1, 4001, "concurrent_test_tool", "{\"client\":\"1\"}"); - k_msleep(200); send_tools_call_request(CLIENT_ID_MULTI_CLIENT_2, 4002, "concurrent_test_tool", "{\"client\":\"2\"}"); - k_msleep(200); /* Both executions should succeed */ zassert_equal(tool_execution_count, 2, "Tool should execute from both clients"); @@ -723,18 +726,15 @@ ZTEST(mcp_server_tests, test_tools_call_edge_cases) long_tool_name[sizeof(long_tool_name) - 1] = '\0'; send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 5001, long_tool_name, "{}"); - k_msleep(200); zassert_equal(tool_execution_count, 0, "Tool with long name should not execute"); /* Test: Empty tool name */ send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 5002, "", "{}"); - k_msleep(200); zassert_equal(tool_execution_count, 0, "Tool with empty name should not execute"); /* Test: Special characters in parameters */ send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 5003, "edge_case_tool", "{\"special\":\"\\\"quotes\\\"\"}"); - k_msleep(200); zassert_equal(tool_execution_count, 1, "Tool with special characters should execute"); /* Clean up */ @@ -771,9 +771,12 @@ ZTEST(mcp_server_tests, test_tools_list_response) reset_transport_mock(); + /* Unregistered client should now get error response */ send_tools_list_request(CLIENT_ID_UNREGISTERED, REQ_ID_EDGE_CASE_UNREGISTERED); - zassert_equal(mcp_transport_queue_call_count, 0, - "Tools/list should be rejected for unregistered client"); + zassert_equal(mcp_transport_queue_call_count, 1, + "Tools/list should send error response for unregistered client"); + zassert_equal(mcp_transport_last_queued_msg.type, MCP_MSG_ERROR_TOOLS_LIST, + "Should receive error response"); mcp_tool_record_t test_tool1 = { .metadata = { @@ -1111,8 +1114,10 @@ ZTEST(mcp_server_tests, test_client_lifecycle) reset_transport_mock(); send_tools_list_request(CLIENT_ID_LIFECYCLE_TEST, REQ_ID_LIFECYCLE_TOOLS_INIT); - zassert_equal(mcp_transport_queue_call_count, 0, - "Tools/list should be rejected before client is initialized"); + zassert_equal(mcp_transport_queue_call_count, 1, + "Tools/list should send error response before client is initialized"); + zassert_equal(mcp_transport_last_queued_msg.type, MCP_MSG_ERROR_TOOLS_LIST, + "Should receive error response"); send_initialized_notification(CLIENT_ID_LIFECYCLE_TEST); @@ -1144,8 +1149,10 @@ ZTEST(mcp_server_tests, test_client_shutdown) reset_transport_mock(); send_tools_list_request(CLIENT_ID_SHUTDOWN_TEST, REQ_ID_SHUTDOWN_TOOLS_DEAD); - zassert_equal(mcp_transport_queue_call_count, 0, - "Tools/list should be rejected for shutdown client"); + zassert_equal(mcp_transport_queue_call_count, 1, + "Tools/list should send error response for shutdown client"); + zassert_equal(mcp_transport_last_queued_msg.type, MCP_MSG_ERROR_TOOLS_LIST, + "Should receive error response"); reset_transport_mock(); send_client_shutdown(CLIENT_ID_UNREGISTERED); @@ -1162,7 +1169,10 @@ ZTEST(mcp_server_tests, test_invalid_states) reset_transport_mock(); send_initialize_request(CLIENT_ID_INVALID_STATE_TEST, REQ_ID_INVALID_REINITIALIZE); - zassert_equal(mcp_transport_queue_call_count, 0, "Re-initialization should be rejected"); + zassert_equal(mcp_transport_queue_call_count, 1, + "Re-initialization should send error response"); + zassert_equal(mcp_transport_last_queued_msg.type, MCP_MSG_ERROR_INITIALIZE, + "Should receive initialize error response"); reset_transport_mock(); send_initialized_notification(CLIENT_ID_UNREGISTERED); @@ -1190,8 +1200,10 @@ ZTEST(mcp_server_tests, test_multiple_client_lifecycle) reset_transport_mock(); send_initialize_request(CLIENT_ID_MULTI_CLIENT_4, REQ_ID_MULTI_CLIENT_4_INIT_1); - zassert_equal(mcp_transport_queue_call_count, 0, - "4th client should be rejected when registry is full"); + zassert_equal(mcp_transport_queue_call_count, 1, + "4th client should send error response when registry is full"); + zassert_equal(mcp_transport_last_queued_msg.type, MCP_MSG_ERROR_INITIALIZE, + "Should receive initialize error response"); send_client_shutdown(CLIENT_ID_MULTI_CLIENT_1); @@ -1221,9 +1233,6 @@ ZTEST(mcp_server_tests, test_unknown_message_type) ret = k_msgq_put(&mcp_request_queue, &msg, K_NO_WAIT); zassert_equal(ret, 0, "Invalid message queueing should succeed"); - - k_msleep(100); - zassert_equal(mcp_transport_queue_call_count, 0, "No response should be sent for unknown message type"); } @@ -1248,9 +1257,6 @@ ZTEST(mcp_server_tests, test_null_data_request) ret = k_msgq_put(&mcp_request_queue, &msg, K_NO_WAIT); zassert_equal(ret, 0, "NULL data message queueing should succeed"); - - k_msleep(100); - zassert_equal(mcp_transport_queue_call_count, 0, "No response should be sent for NULL data pointer"); } @@ -1380,8 +1386,6 @@ ZTEST(mcp_server_tests, test_invalid_execution_tokens) /* Execute the tool - this will create and then destroy an execution token */ send_tools_call_request(CLIENT_ID_EDGE_CASE_TEST, 4500, "token_test_tool", "{}"); - k_msleep(200); /* Wait for tool execution to complete */ - zassert_equal(tool_execution_count, 1, "Tool should have executed once"); uint32_t used_token = last_execution_token; From 26a81305c9ae3a66a78fba446b703e464a2d5173 Mon Sep 17 00:00:00 2001 From: David Piskula Date: Tue, 25 Nov 2025 10:16:24 +0100 Subject: [PATCH 10/12] networking: improved error codes Defined an enum for error codes Signed-off-by: David Piskula --- subsys/net/lib/mcp/mcp_common.h | 10 ++++++++ subsys/net/lib/mcp/mcp_server.c | 42 +++++++++++++++------------------ 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/subsys/net/lib/mcp/mcp_common.h b/subsys/net/lib/mcp/mcp_common.h index 9ccd2a40d7d47..f373ca651c1cd 100644 --- a/subsys/net/lib/mcp/mcp_common.h +++ b/subsys/net/lib/mcp/mcp_common.h @@ -57,6 +57,16 @@ typedef enum { MCP_EXEC_ZOMBIE } mcp_execution_state_t; +typedef enum { + /* JSON-RPC 2.0 standard error codes */ + MCP_ERROR_PARSE_ERROR = -32700, + MCP_ERROR_INVALID_REQUEST = -32600, + MCP_ERROR_METHOD_NOT_FOUND = -32601, + MCP_ERROR_INVALID_PARAMS = -32602, + MCP_ERROR_INTERNAL_ERROR = -32603, + MCP_ERROR_SERVER_ERROR = -32000, +} mcp_error_code_t; + typedef struct mcp_system_msg { mcp_system_msg_type_t type; uint32_t request_id; diff --git a/subsys/net/lib/mcp/mcp_server.c b/subsys/net/lib/mcp/mcp_server.c index 53ac1f6e71894..a426ec803e9c8 100644 --- a/subsys/net/lib/mcp/mcp_server.c +++ b/subsys/net/lib/mcp/mcp_server.c @@ -693,11 +693,10 @@ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) ret = handle_initialize_request(init_request); if (ret != 0) { LOG_ERR("Initialize request failed: %d", ret); - /* Send error response according to JSON-RPC spec */ - send_error_response(init_request->request_id, - MCP_MSG_ERROR_INITIALIZE, - -32603, /* Internal error */ - "Server initialization failed"); + send_error_response(init_request->request_id, + MCP_MSG_ERROR_INITIALIZE, + MCP_ERROR_INTERNAL_ERROR, + "Server initialization failed"); } mcp_free(init_request); break; @@ -711,24 +710,21 @@ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) ret = handle_tools_list_request(tools_list_request); if (ret != 0) { LOG_ERR("Tools list request failed: %d", ret); - const char *error_msg; + int32_t error_code; + const char *error_msg; - if (ret == -ENOENT) { - error_code = -32602; /* Invalid params */ + if (ret == -ENOENT || ret == -EPERM) { + error_code = MCP_ERROR_INVALID_PARAMS; error_msg = "Client not found or not initialized"; - } else if (ret == -EPERM) { - error_code = -32602; /* Invalid params */ - error_msg = - "Client not in correct state for tools requests"; } else { - error_code = -32603; /* Internal error */ + error_code = MCP_ERROR_INTERNAL_ERROR; error_msg = "Internal server error processing tools list"; } send_error_response(tools_list_request->request_id, - MCP_MSG_ERROR_TOOLS_LIST, error_code, - error_msg); + MCP_MSG_ERROR_TOOLS_LIST, + error_code, error_msg); } mcp_free(tools_list_request); break; @@ -741,26 +737,27 @@ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) ret = handle_tools_call_request(tools_call_request); if (ret != 0) { LOG_ERR("Tools call request failed: %d", ret); - const char *error_msg; + int32_t error_code; + const char *error_msg; if (ret == -ENOENT) { - error_code = -32601; /* Method not found */ + error_code = MCP_ERROR_METHOD_NOT_FOUND; error_msg = "Tool not found"; } else if (ret == -EPERM) { - error_code = -32602; /* Invalid params */ + error_code = MCP_ERROR_INVALID_PARAMS; error_msg = "Client not authorized for tool execution"; } else if (ret == -ENOSPC) { - error_code = -32603; /* Internal error */ + error_code = MCP_ERROR_SERVER_ERROR; error_msg = "No available execution slots"; } else { - error_code = -32603; /* Internal error */ + error_code = MCP_ERROR_INTERNAL_ERROR; error_msg = "Internal server error processing tool call"; } send_error_response(tools_call_request->request_id, - MCP_MSG_ERROR_TOOLS_CALL, error_code, - error_msg); + MCP_MSG_ERROR_TOOLS_CALL, + error_code, error_msg); } mcp_free(tools_call_request); break; @@ -1026,7 +1023,6 @@ int mcp_server_submit_app_message(const mcp_app_message_t *app_msg, uint32_t exe ret); return ret; } - /* TODO: Any way to wait until request worker is released? */ } return 0; From 535cc291d1147b511855ac4e2a4b3a7725b33aff Mon Sep 17 00:00:00 2001 From: David Piskula Date: Tue, 25 Nov 2025 13:26:59 +0100 Subject: [PATCH 11/12] networking: worker improvements Refactored code, made response workers optional Signed-off-by: David Piskula --- subsys/net/lib/mcp/Kconfig | 30 ++++++++++++++++ subsys/net/lib/mcp/mcp_server.c | 62 ++++++++++++++++++--------------- tests/net/lib/mcp/prj.conf | 1 + 3 files changed, 64 insertions(+), 29 deletions(-) diff --git a/subsys/net/lib/mcp/Kconfig b/subsys/net/lib/mcp/Kconfig index e474d9c89b571..d8aaf3551a6b8 100644 --- a/subsys/net/lib/mcp/Kconfig +++ b/subsys/net/lib/mcp/Kconfig @@ -23,6 +23,36 @@ config HTTP_SERVER_MAX_STREAMS help This setting determines the maximum number of HTTP/2 streams for each client. +config MCP_REQUEST_WORKERS + int "Number of MCP request worker threads" + default 2 + range 1 8 + help + Number of worker threads that process incoming MCP requests. + More workers allow better concurrent handling of client requests. + +config MCP_RESPONSE_WORKERS + int "Number of MCP message worker threads" + default 2 + range 0 8 + help + Number of worker threads that process outgoing MCP responses. + Set to 0 to disable response workers and process responses directly. + +config MCP_REQUEST_QUEUE_SIZE + int "MCP request queue size" + default 5 + range 1 100 + help + Maximum number of pending MCP requests that can be queued for processing. + +config MCP_RESPONSE_QUEUE_SIZE + int "MCP response queue size" + default 5 + range 1 100 + help + Maximum number of pending MCP responses that can be queued for sending. + config MCP_SERVER_INFO_NAME_MAX_LEN int "Maximum server info name length" default 64 diff --git a/subsys/net/lib/mcp/mcp_server.c b/subsys/net/lib/mcp/mcp_server.c index a426ec803e9c8..e4eb7e1952e7e 100644 --- a/subsys/net/lib/mcp/mcp_server.c +++ b/subsys/net/lib/mcp/mcp_server.c @@ -13,10 +13,6 @@ LOG_MODULE_REGISTER(mcp_server, CONFIG_MCP_LOG_LEVEL); -#define MCP_REQUEST_WORKERS 2 -#define MCP_MESSAGE_WORKERS 2 -#define MCP_REQUEST_QUEUE_SIZE 10 -#define MCP_MESSAGE_QUEUE_SIZE 10 #define MCP_WORKER_PRIORITY 7 typedef enum { @@ -44,16 +40,16 @@ static mcp_client_registry_t client_registry; static mcp_tool_registry_t tool_registry; static mcp_execution_registry_t execution_registry; -K_MSGQ_DEFINE(mcp_request_queue, sizeof(mcp_request_queue_msg_t), MCP_REQUEST_QUEUE_SIZE, 4); -K_MSGQ_DEFINE(mcp_message_queue, sizeof(mcp_response_queue_msg_t), MCP_MESSAGE_QUEUE_SIZE, 4); +K_MSGQ_DEFINE(mcp_request_queue, sizeof(mcp_request_queue_msg_t), CONFIG_MCP_REQUEST_QUEUE_SIZE, 4); +K_THREAD_STACK_ARRAY_DEFINE(mcp_request_worker_stacks, CONFIG_MCP_REQUEST_WORKERS, 2048); +static struct k_thread mcp_request_workers[CONFIG_MCP_REQUEST_WORKERS]; -K_THREAD_STACK_ARRAY_DEFINE(mcp_request_worker_stacks, MCP_REQUEST_WORKERS, 2048); -K_THREAD_STACK_ARRAY_DEFINE(mcp_message_worker_stacks, MCP_MESSAGE_WORKERS, 2048); - -static struct k_thread mcp_request_workers[MCP_REQUEST_WORKERS]; -static struct k_thread mcp_message_workers[MCP_MESSAGE_WORKERS]; - -/* Helper functions */ +#if CONFIG_MCP_RESPONSE_WORKERS > 0 +K_MSGQ_DEFINE(mcp_response_queue, sizeof(mcp_response_queue_msg_t), CONFIG_MCP_RESPONSE_QUEUE_SIZE, + 4); +K_THREAD_STACK_ARRAY_DEFINE(mcp_response_worker_stacks, CONFIG_MCP_RESPONSE_WORKERS, 2048); +static struct k_thread mcp_response_workers[CONFIG_MCP_RESPONSE_WORKERS]; +#endif static int find_client_index(uint32_t client_id) { @@ -783,6 +779,7 @@ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) } } +#if CONFIG_MCP_RESPONSE_WORKERS > 0 static void mcp_response_worker(void *arg1, void *arg2, void *arg3) { mcp_response_queue_msg_t response; @@ -792,7 +789,7 @@ static void mcp_response_worker(void *arg1, void *arg2, void *arg3) LOG_INF("Response worker %d started", worker_id); while (1) { - ret = k_msgq_get(&mcp_message_queue, &response, K_FOREVER); + ret = k_msgq_get(&mcp_response_queue, &response, K_FOREVER); if (ret == 0) { LOG_DBG("Processing response (worker %d)", worker_id); /* TODO: When Server-sent Events and Authorization are added, implement the @@ -801,9 +798,6 @@ static void mcp_response_worker(void *arg1, void *arg2, void *arg3) /* In phase 1, the response worker queue and workers are pretty much * redundant by design */ - /* TODO: Consider making response workers a configurable resource that can - * be bypassed - */ ret = mcp_transport_queue_response(response.type, response.data); if (ret != 0) { LOG_ERR("Failed to queue response to transport: %d", ret); @@ -814,8 +808,7 @@ static void mcp_response_worker(void *arg1, void *arg2, void *arg3) } } } - -/* Public API functions */ +#endif int mcp_server_init(void) { @@ -857,7 +850,7 @@ int mcp_server_start(void) LOG_INF("Starting MCP Server"); - for (int i = 0; i < MCP_REQUEST_WORKERS; i++) { + for (int i = 0; i < CONFIG_MCP_REQUEST_WORKERS; i++) { tid = k_thread_create(&mcp_request_workers[i], mcp_request_worker_stacks[i], K_THREAD_STACK_SIZEOF(mcp_request_worker_stacks[i]), mcp_request_worker, INT_TO_POINTER(i), NULL, NULL, @@ -873,24 +866,26 @@ int mcp_server_start(void) } } - for (int i = 0; i < MCP_MESSAGE_WORKERS; i++) { - tid = k_thread_create(&mcp_message_workers[i], mcp_message_worker_stacks[i], - K_THREAD_STACK_SIZEOF(mcp_message_worker_stacks[i]), +#if CONFIG_MCP_RESPONSE_WORKERS > 0 + for (int i = 0; i < CONFIG_MCP_RESPONSE_WORKERS; i++) { + tid = k_thread_create(&mcp_response_workers[i], mcp_response_worker_stacks[i], + K_THREAD_STACK_SIZEOF(mcp_response_worker_stacks[i]), mcp_response_worker, INT_TO_POINTER(i), NULL, NULL, K_PRIO_COOP(MCP_WORKER_PRIORITY), 0, K_NO_WAIT); if (tid == NULL) { - LOG_ERR("Failed to create message worker %d", i); + LOG_ERR("Failed to create response worker %d", i); return -ENOMEM; } - ret = k_thread_name_set(&mcp_message_workers[i], "mcp_msg_worker"); + ret = k_thread_name_set(&mcp_response_workers[i], "mcp_msg_worker"); if (ret != 0) { LOG_WRN("Failed to set thread name: %d", ret); } } +#endif - LOG_INF("MCP Server started: %d request, %d message workers", MCP_REQUEST_WORKERS, - MCP_MESSAGE_WORKERS); + LOG_INF("MCP Server started: %d request, %d response workers", CONFIG_MCP_REQUEST_WORKERS, + CONFIG_MCP_RESPONSE_WORKERS); return 0; } @@ -975,13 +970,22 @@ int mcp_server_submit_app_message(const mcp_app_message_t *app_msg, uint32_t exe return -EINVAL; } +#if CONFIG_MCP_RESPONSE_WORKERS > 0 /* TODO: Make response workers configurable? If not used, call transport API directly? */ - ret = k_msgq_put(&mcp_message_queue, &response, K_NO_WAIT); + ret = k_msgq_put(&mcp_response_queue, &response, K_NO_WAIT); + if (ret != 0) { + LOG_ERR("Failed to submit response to response queue: %d", ret); + mcp_free(response.data); + return ret; + } +#else + ret = mcp_transport_queue_response(response.type, response.data); if (ret != 0) { - LOG_ERR("Failed to submit response to message queue: %d", ret); + LOG_ERR("Failed to queue response to transport: %d", ret); mcp_free(response.data); return ret; } +#endif if (app_msg->type == MCP_USR_TOOL_RESPONSE) { ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); diff --git a/tests/net/lib/mcp/prj.conf b/tests/net/lib/mcp/prj.conf index e042512570ccf..11b6566686213 100644 --- a/tests/net/lib/mcp/prj.conf +++ b/tests/net/lib/mcp/prj.conf @@ -3,6 +3,7 @@ CONFIG_MCP_SERVER=y CONFIG_MCP_TOOLS_CAPABILITY=y CONFIG_MCP_MAX_TOOLS=4 CONFIG_MCP_TOOL_DESC=y +CONFIG_MCP_RESPONSE_WORKERS=2 CONFIG_MCP_SERVER_INFO_TITLE=y CONFIG_MCP_SERVER_INFO_INSTRUCTIONS=y CONFIG_MCP_TOOL_DESC=y From f33f38a9772171798fc88f6c270ef2fb879b9a1c Mon Sep 17 00:00:00 2001 From: David Piskula Date: Tue, 25 Nov 2025 13:44:44 +0100 Subject: [PATCH 12/12] networking: additional init error Additional error handling for client initialization Signed-off-by: David Piskula --- subsys/net/lib/mcp/mcp_server.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/subsys/net/lib/mcp/mcp_server.c b/subsys/net/lib/mcp/mcp_server.c index e4eb7e1952e7e..897e0b8e2c651 100644 --- a/subsys/net/lib/mcp/mcp_server.c +++ b/subsys/net/lib/mcp/mcp_server.c @@ -342,7 +342,6 @@ static int handle_initialize_request(mcp_initialize_request_t *request) int ret; LOG_DBG("Processing initialize request"); - /* TODO: Handle sending error responses to client */ ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); if (ret != 0) { @@ -467,7 +466,6 @@ static int handle_tools_list_request(mcp_tools_list_request_t *request) int client_index; int ret; - /* TODO: Handle sending error responses to client */ LOG_DBG("Processing tools list request"); ret = k_mutex_lock(&client_registry.registry_mutex, K_FOREVER); @@ -689,10 +687,17 @@ static void mcp_request_worker(void *arg1, void *arg2, void *arg3) ret = handle_initialize_request(init_request); if (ret != 0) { LOG_ERR("Initialize request failed: %d", ret); - send_error_response(init_request->request_id, - MCP_MSG_ERROR_INITIALIZE, - MCP_ERROR_INTERNAL_ERROR, - "Server initialization failed"); + if (ret == -EALREADY) { + send_error_response( + init_request->request_id, MCP_MSG_ERROR_INITIALIZE, + MCP_ERROR_INVALID_PARAMS, + "Client already initialized or in invalid state"); + } else { + send_error_response(init_request->request_id, + MCP_MSG_ERROR_INITIALIZE, + MCP_ERROR_INTERNAL_ERROR, + "Server initialization failed"); + } } mcp_free(init_request); break;