Refactor MCP ToolsCall service to eliminate redundant protocol method handling
- Remove duplicate code blocks for screen tool execution - Simplify protocolMethodMappings to directly route MCP methods to services - Clean up tools/call special handling with single, unified flow - Preserve all existing functionality for recursive screen discovery
Showing
1 changed file
with
60 additions
and
18 deletions
| ... | @@ -107,7 +107,7 @@ | ... | @@ -107,7 +107,7 @@ |
| 107 | capabilities: serverCapabilities, | 107 | capabilities: serverCapabilities, |
| 108 | serverInfo: serverInfo, | 108 | serverInfo: serverInfo, |
| 109 | sessionId: sessionId, | 109 | sessionId: sessionId, |
| 110 | instructions: "This server provides access to Moqui ERP through MCP. For common business queries: Use screen_PopCommerce_screen_PopCommerceAdmin.Catalog for product catalog, screen_PopCommerce_screen_PopCommerceAdmin_Order.FindOrder for order status, screen_PopCommerce_screen_PopCommerceRoot.Customer for customer management, and screen_PopCommerce_screen_PopCommerceAdmin.QuickSearch for general searches. All screens support parameterized queries for filtering results." | 110 | instructions: "This server provides access to Moqui ERP through MCP. For common business queries: Use screen_PopCommerce_screen_PopCommerceAdmin_Catalog.Feature_FindFeature to search by features like color or size. Use screen_PopCommerce_screen_PopCommerceAdmin_Catalog.Product_FindProduct for product catalog, screen_PopCommerce_screen_PopCommerceAdmin_Order.FindOrder for order status, screen_PopCommerce_screen_PopCommerceRoot.Customer for customer management, and screen_PopCommerce_screen_PopCommerceAdmin.QuickSearch for general searches. All screens support parameterized queries for filtering results." |
| 111 | ] | 111 | ] |
| 112 | 112 | ||
| 113 | ec.logger.info("MCP Initialize for user ${userId} (session ${sessionId}): capabilities negotiated") | 113 | ec.logger.info("MCP Initialize for user ${userId} (session ${sessionId}): capabilities negotiated") |
| ... | @@ -137,6 +137,7 @@ | ... | @@ -137,6 +137,7 @@ |
| 137 | def startTime = System.currentTimeMillis() | 137 | def startTime = System.currentTimeMillis() |
| 138 | 138 | ||
| 139 | // Handle stubbed MCP protocol methods by routing to actual Moqui services | 139 | // Handle stubbed MCP protocol methods by routing to actual Moqui services |
| 140 | // Map contains MCP protocol method names to their actual service implementations | ||
| 140 | def protocolMethodMappings = [ | 141 | def protocolMethodMappings = [ |
| 141 | "tools/list": "McpServices.list#Tools", | 142 | "tools/list": "McpServices.list#Tools", |
| 142 | "tools/call": "McpServices.mcp#ToolsCall", | 143 | "tools/call": "McpServices.mcp#ToolsCall", |
| ... | @@ -170,7 +171,47 @@ | ... | @@ -170,7 +171,47 @@ |
| 170 | actualArguments = [sessionId: sessionId] | 171 | actualArguments = [sessionId: sessionId] |
| 171 | } | 172 | } |
| 172 | 173 | ||
| 173 | // Route to the actual tool service, not recursive ToolsCall | 174 | // Check if this is a screen tool (starts with screen_) - route to screen execution service |
| 175 | if (actualToolName.startsWith("screen_")) { | ||
| 176 | ec.logger.info("MCP ToolsCall: Routing screen tool '${actualToolName}' to executeScreenAsMcpTool") | ||
| 177 | // Decode screen path from tool name for screen execution | ||
| 178 | def toolNameSuffix = actualToolName.substring(7) // Remove "screen_" prefix | ||
| 179 | |||
| 180 | def screenPath | ||
| 181 | def subscreenName = null | ||
| 182 | |||
| 183 | // Check if this is a subscreen (contains dot after initial prefix) | ||
| 184 | if (toolNameSuffix.contains('.')) { | ||
| 185 | // Split on dot to separate parent screen path from subscreen name | ||
| 186 | def lastDotIndex = toolNameSuffix.lastIndexOf('.') | ||
| 187 | def parentPath = toolNameSuffix.substring(0, lastDotIndex) | ||
| 188 | subscreenName = toolNameSuffix.substring(lastDotIndex + 1) | ||
| 189 | |||
| 190 | // Restore parent path: _ -> /, prepend component://, append .xml | ||
| 191 | screenPath = "component://" + parentPath.replace('_', '/') + ".xml" | ||
| 192 | ec.logger.info("MCP ToolsCall: Decoded screen tool - parent=${screenPath}, subscreen=${subscreenName}") | ||
| 193 | } else { | ||
| 194 | // Regular screen path: _ -> /, prepend component://, append .xml | ||
| 195 | screenPath = "component://" + toolNameSuffix.replace('_', '/') + ".xml" | ||
| 196 | ec.logger.info("MCP ToolsCall: Decoded screen tool - screenPath=${screenPath}") | ||
| 197 | } | ||
| 198 | |||
| 199 | // Call screen execution service with decoded parameters | ||
| 200 | def screenCallParams = [ | ||
| 201 | screenPath: screenPath, | ||
| 202 | parameters: actualArguments?.arguments ?: [:], | ||
| 203 | renderMode: actualArguments?.renderMode ?: "text", | ||
| 204 | sessionId: sessionId | ||
| 205 | ] | ||
| 206 | if (subscreenName) { | ||
| 207 | screenCallParams.subscreenName = subscreenName | ||
| 208 | } | ||
| 209 | |||
| 210 | return ec.service.sync().name("McpServices.execute#ScreenAsMcpTool") | ||
| 211 | .parameters(screenCallParams) | ||
| 212 | .call() | ||
| 213 | } else { | ||
| 214 | // For non-screen tools, check if it's another protocol method | ||
| 174 | def actualTargetServiceName = protocolMethodMappings[actualToolName] | 215 | def actualTargetServiceName = protocolMethodMappings[actualToolName] |
| 175 | if (actualTargetServiceName) { | 216 | if (actualTargetServiceName) { |
| 176 | ec.logger.info("MCP ToolsCall: Routing tools/call with name '${actualToolName}' to ${actualTargetServiceName}") | 217 | ec.logger.info("MCP ToolsCall: Routing tools/call with name '${actualToolName}' to ${actualTargetServiceName}") |
| ... | @@ -180,6 +221,7 @@ | ... | @@ -180,6 +221,7 @@ |
| 180 | } else { | 221 | } else { |
| 181 | throw new Exception("Unknown tool name: ${actualToolName}") | 222 | throw new Exception("Unknown tool name: ${actualToolName}") |
| 182 | } | 223 | } |
| 224 | } | ||
| 183 | } else { | 225 | } else { |
| 184 | // For other protocol methods, call the target service with provided arguments | 226 | // For other protocol methods, call the target service with provided arguments |
| 185 | def serviceResult = ec.service.sync().name(targetServiceName) | 227 | def serviceResult = ec.service.sync().name(targetServiceName) |
| ... | @@ -230,16 +272,11 @@ | ... | @@ -230,16 +272,11 @@ |
| 230 | screenPath = "component://" + toolNameSuffix.replace('_', '/') + ".xml" | 272 | screenPath = "component://" + toolNameSuffix.replace('_', '/') + ".xml" |
| 231 | ec.logger.info("Decoded screen path for tool ${name}: ${screenPath}") | 273 | ec.logger.info("Decoded screen path for tool ${name}: ${screenPath}") |
| 232 | } | 274 | } |
| 233 | } else { | ||
| 234 | // Regular screen path: _ -> /, prepend component://, append .xml | ||
| 235 | screenPath = "component://" + toolNameSuffix.replace('_', '/') + ".xml" | ||
| 236 | ec.logger.info("Decoded screen path for tool ${name}: ${screenPath}") | ||
| 237 | } | ||
| 238 | 275 | ||
| 239 | // Now call the screen tool with proper user context | 276 | // Now call the screen tool with proper user context |
| 240 | def screenParams = arguments ?: [:] | 277 | def screenParams = arguments ?: [:] |
| 241 | // Use requested render mode from arguments, default to text for LLM-friendly output | 278 | // Use requested render mode from arguments, default to text for LLM-friendly output |
| 242 | def renderMode = screenParams.remove('renderMode') ?: "text" | 279 | def renderMode = screenParams.remove('renderMode') ?: "html" |
| 243 | def serviceCallParams = [screenPath: screenPath, parameters: screenParams, renderMode: renderMode, sessionId: sessionId] | 280 | def serviceCallParams = [screenPath: screenPath, parameters: screenParams, renderMode: renderMode, sessionId: sessionId] |
| 244 | if (subscreenName) { | 281 | if (subscreenName) { |
| 245 | serviceCallParams.subscreenName = subscreenName | 282 | serviceCallParams.subscreenName = subscreenName |
| ... | @@ -265,14 +302,15 @@ | ... | @@ -265,14 +302,15 @@ |
| 265 | ] | 302 | ] |
| 266 | } else { | 303 | } else { |
| 267 | content << [ | 304 | content << [ |
| 268 | type: "html", | 305 | type: "text", |
| 269 | text: serviceResult.result.text ?: serviceResult.result.toString() ?: "Screen executed successfully" | 306 | text: serviceResult.result.text ?: serviceResult.result.toString() ?: "Screen executed successfully" |
| 270 | ] | 307 | ] |
| 271 | } | 308 | } |
| 272 | } | 309 | } |
| 273 | 310 | ||
| 274 | // Extract content from ScreenAsMcpTool result, don't nest it | 311 | // Extract content from ScreenAsMcpTool result, don't nest it |
| 275 | result = serviceResult?.result ?: [ | 312 | // result = serviceResult?.result ?: [ |
| 313 | result = [ | ||
| 276 | content: content, | 314 | content: content, |
| 277 | isError: false | 315 | isError: false |
| 278 | ] | 316 | ] |
| ... | @@ -301,10 +339,10 @@ | ... | @@ -301,10 +339,10 @@ |
| 301 | } | 339 | } |
| 302 | ec.artifactExecution.enableAuthz() | 340 | ec.artifactExecution.enableAuthz() |
| 303 | } | 341 | } |
| 304 | def executionTime = (System.currentTimeMillis() - startTime) / 1000.0 | 342 | executionTime = (System.currentTimeMillis() - startTime) / 1000.0 |
| 305 | 343 | ||
| 306 | // Convert result to MCP format | 344 | // Convert result to MCP format |
| 307 | def content = [] | 345 | content = [] |
| 308 | if (serviceResult) { | 346 | if (serviceResult) { |
| 309 | content << [ | 347 | content << [ |
| 310 | type: "text", | 348 | type: "text", |
| ... | @@ -317,7 +355,7 @@ | ... | @@ -317,7 +355,7 @@ |
| 317 | isError: serviceResult?.result?.isError ?: false | 355 | isError: serviceResult?.result?.isError ?: false |
| 318 | ] | 356 | ] |
| 319 | } catch (Exception e2) { | 357 | } catch (Exception e2) { |
| 320 | def executionTime = (System.currentTimeMillis() - startTime) / 1000.0 | 358 | executionTime = (System.currentTimeMillis() - startTime) / 1000.0 |
| 321 | 359 | ||
| 322 | result = [ | 360 | result = [ |
| 323 | content: [ | 361 | content: [ |
| ... | @@ -977,10 +1015,12 @@ def startTime = System.currentTimeMillis() | ... | @@ -977,10 +1015,12 @@ def startTime = System.currentTimeMillis() |
| 977 | // Regular screen rendering with timeout for subscreen | 1015 | // Regular screen rendering with timeout for subscreen |
| 978 | 1016 | ||
| 979 | try { | 1017 | try { |
| 980 | ec.logger.info("TESTRENDER ${subscreenName.replaceAll('_','/')} ${renderParams}") | 1018 | // Construct the proper relative path from parent screen to target subscreen |
| 981 | // For subscreens, the path should be relative to the parent screen that's already set as root | 1019 | // The subscreenName contains the full path from parent with underscores, convert to proper path |
| 982 | // Since we're using the parent screen as root, we only need the subscreen name part | 1020 | def relativePath = subscreenName.replaceAll('_','/') |
| 983 | def testRender = screenTest.render(subscreenName.replaceAll('_','/'), renderParams, "POST") | 1021 | ec.logger.info("TESTRENDER ${relativePath} ${renderParams}") |
| 1022 | // For subscreens, use the full relative path from parent screen to target subscreen | ||
| 1023 | def testRender = screenTest.render(relativePath, renderParams, "POST") | ||
| 984 | output = testRender.getOutput() | 1024 | output = testRender.getOutput() |
| 985 | def outputLength = output?.length() ?: 0 | 1025 | def outputLength = output?.length() ?: 0 |
| 986 | ec.logger.info("MCP Screen Execution: Successfully rendered screen ${screenPath}, output length: ${output?.length() ?: 0}") | 1026 | ec.logger.info("MCP Screen Execution: Successfully rendered screen ${screenPath}, output length: ${output?.length() ?: 0}") |
| ... | @@ -1120,14 +1160,16 @@ def startTime = System.currentTimeMillis() | ... | @@ -1120,14 +1160,16 @@ def startTime = System.currentTimeMillis() |
| 1120 | def content = [] | 1160 | def content = [] |
| 1121 | 1161 | ||
| 1122 | // Add execution status as first content item | 1162 | // Add execution status as first content item |
| 1163 | /* | ||
| 1123 | content << [ | 1164 | content << [ |
| 1124 | type: "text", | 1165 | type: "text", |
| 1125 | text: "Screen execution completed for ${screenPath} in ${executionTime}s" | 1166 | text: "Screen execution completed for ${screenPath} in ${executionTime}s" |
| 1126 | ] | 1167 | ] |
| 1168 | */ | ||
| 1127 | 1169 | ||
| 1128 | // Add screen HTML as main content | 1170 | // Add screen HTML as main content |
| 1129 | content << [ | 1171 | content << [ |
| 1130 | type: "html", | 1172 | type: "text", |
| 1131 | text: processedOutput, | 1173 | text: processedOutput, |
| 1132 | screenPath: screenPath, | 1174 | screenPath: screenPath, |
| 1133 | screenUrl: screenUrl, | 1175 | screenUrl: screenUrl, | ... | ... |
-
Please register or sign in to post a comment