Fix MCP screen discovery closure and JSON-RPC response nesting

- Separate processScreenWithSubscreens closure definition to fix Groovy closure scope issues - Add proper flattening of subScreenPathList to handle nested collections - Fix subscreen tool naming with dot notation for parent.child relationships - Enhance screen tool execution to support subscreen parameters - Unwrap Moqui service results in EnhancedMcpServlet to avoid double nesting in JSON-RPC responses - Improve error handling and logging throughout screen discovery process Now successfully discovers 29 total tools (17 screen tools + 12 service tools) with proper session management.

Fix MCP screen discovery closure and JSON-RPC response nesting
- Separate processScreenWithSubscreens closure definition to fix Groovy closure scope issues - Add proper flattening of subScreenPathList to handle nested collections - Fix subscreen tool naming with dot notation for parent.child relationships - Enhance screen tool execution to support subscreen parameters - Unwrap Moqui service results in EnhancedMcpServlet to avoid double nesting in JSON-RPC responses - Improve error handling and logging throughout screen discovery process Now successfully discovers 29 total tools (17 screen tools + 12 service tools) with proper session management.
Ean Schuessler
Showing 3 changed files with 247 additions and 219 deletions
service/McpServices.xml
src/main/groovy/org/moqui/mcp/CustomScreenTestImpl.groovy
src/main/groovy/org/moqui/mcp/EnhancedMcpServlet.groovy
--- a/service/McpServices.xml
View file @e18de6c
+++ b/service/McpServices.xml
View file @e18de6c
@@ -407,61 +407,38 @@
                def isScreenTool = name.startsWith("screen_")
                if (isScreenTool) {
-                    // Decode screen path from tool name (Clean Encoding)
+                    // Decode screen path from tool name (Clean Encoding with subscreen support)
                    def toolNameSuffix = name.substring(7)  // Remove "screen_" prefix
-                    // Restore path: _ -> /, prepend component://, append .xml
+                    def screenPath
-                    def screenPath = "component://" + toolNameSuffix.replace('_', '/') + ".xml"
+                    def subscreenName = null
-                    ec.logger.info("Decoded screen path for tool ${name}: ${screenPath}")
+                    // Check if this is a subscreen (contains dot after the initial prefix)
-                    // Restore user context from sessionId before calling screen tool
+                    if (toolNameSuffix.contains('.')) {
-                    def serviceResult = null
+                        // Split on dot to separate parent screen path from subscreen name
-                    def visit = null
+                        def lastDotIndex = toolNameSuffix.lastIndexOf('.')
-                    UserInfo restoredUserInfo = null
+                        def parentPath = toolNameSuffix.substring(0, lastDotIndex)
-                    try {
+                        subscreenName = toolNameSuffix.substring(lastDotIndex + 1)
-                        // Get Visit to find the actual user who created this session
-                        if (sessionId) {
-                            visit = ec.entity.find("moqui.server.Visit")
-                                .condition("visitId", sessionId)
-                                .disableAuthz()
-                                .one()
-                            if (!visit) {
-                                throw new Exception("Invalid session for screen tool execution: ${sessionId}")
-                            }
-                        } else {
-                            // If no sessionId, we're likely in a test/debug scenario, use current user context
-                            ec.logger.warn("No sessionId provided for screen tool execution, using current user context")
-                            visit = null // Explicitly set to null to indicate no session context
-                        }
-                        // Restore user context - handle special MCP case where Visit was created with ADMIN
-                        if (visit && visit.userId && visit.userId != ec.user.userId) {
-                            // Restore the actual user who created the session
-                            def userAccount = ec.entity.find("moqui.security.UserAccount")
-                                .condition("userId", visit.userId)
-                                .disableAuthz()
-                                .one()
-                            if (userAccount) {
-                                restoredUserInfo = ec.user.pushUser(userAccount.username)
-                                ec.logger.info("Screen tool execution: Restored user context for ${userAccount.username}")
-                            }
-                        }
+                        // Restore parent path: _ -> /, prepend component://, append .xml
+                        screenPath = "component://" + parentPath.replace('_', '/') + ".xml"
+                        ec.logger.info("Decoded subscreen path for tool ${name}: parent=${screenPath}, subscreen=${subscreenName}")
+                    } else {
+                        // Regular screen path: _ -> /, prepend component://, append .xml
+                        screenPath = "component://" + toolNameSuffix.replace('_', '/') + ".xml"
+                        ec.logger.info("Decoded screen path for tool ${name}: ${screenPath}")
+                    }
                        // Now call the screen tool with proper user context
                        def screenParams = arguments ?: [:]
+                        def serviceCallParams = [screenPath: screenPath, parameters: screenParams, renderMode: "html", sessionId: sessionId]
+                        if (subscreenName) {
+                            serviceCallParams.subscreenName = subscreenName
+                        }
                        serviceResult = ec.service.sync().name("McpServices.execute#ScreenAsMcpTool")
-                            .parameters([screenPath: screenPath, parameters: screenParams, renderMode: "html", sessionId: sessionId])
+                            .parameters(serviceCallParams)
                            .call()
-                    } finally {
-                        // Always restore original user context
-                        if (restoredUserInfo != null) {
-                            ec.user.popUser()
-                            ec.logger.info("Screen tool execution: Restored original user context ${ec.user.username}")
-                        }
-                    }
                    def executionTime = (System.currentTimeMillis() - startTime) / 1000.0
                    // Convert result to MCP format
@@ -476,7 +453,7 @@
                            ]
                        } else {
                            content << [
-                                type: "text", 
+                                type: "html", 
                                text: serviceResult.result.toString() ?: "Screen executed successfully"
                            ]
                        }
@@ -525,7 +502,7 @@
                        content: content,
                        isError: false
                    ]
-                } catch (Exception e) {
+                } catch (Exception e2) {
                    def executionTime = (System.currentTimeMillis() - startTime) / 1000.0
                    result = [
@@ -569,14 +546,6 @@
                def resources = []
                UserInfo adminUserInfo = null
-                // Update session activity
-                def metadata = [:]
-                try {
-                   metadata = groovy.json.JsonSlurper().parseText(visit.initialRequest ?: "{}") as Map
-                } catch (Exception e) {
-                   ec.logger.debug("Failed to parse Visit metadata: ${e.message}")
-                }
                // Store original user context before switching to ADMIN
                def originalUsername = ec.user.username
@@ -999,7 +968,7 @@ try {
                def tools = []
-// Discover screens that user can actually access
+                // Discover screens that user can actually access
                def accessibleScreens = [] as Set<String>
                adminUserInfo = null
                try {
@@ -1039,6 +1008,25 @@ try {
                    return "screen_" + cleanPath.replace('/', '_')
                }
+                // Helper function to convert screen path to MCP tool name with subscreen support
+                def screenPathToToolNameWithSubscreens = { screenPath, parentScreenPath = null ->
+                    // If we have a parent screen path, this is a subscreen - use dot notation
+                    if (parentScreenPath) {
+                        def parentCleanPath = parentScreenPath
+                        if (parentCleanPath.startsWith("component://")) parentCleanPath = parentCleanPath.substring(12)
+                        if (parentCleanPath.endsWith(".xml")) parentCleanPath = parentCleanPath.substring(0, parentCleanPath.length() - 4)
+                        // Extract subscreen name from the full screen path
+                        def subscreenName = screenPath.split("/")[-1]
+                        if (subscreenName.endsWith(".xml")) subscreenName = subscreenName.substring(0, subscreenName.length() - 4)
+                        return "screen_" + parentCleanPath.replace('/', '_') + "." + subscreenName
+                    }
+                    // Regular screen path conversion for main screens
+                    return screenPathToToolName(screenPath)
+                }
                // Helper function to create MCP tool from screen
                def createScreenTool = { screenPath, title, description, parameters = [:] ->
                    def toolName = screenPathToToolName(screenPath)
@@ -1054,98 +1042,45 @@ try {
                    ]
                }
-                // Helper function to recursively discover subscreens
+                // Helper function to recursively process screens and create tools directly
-                def discoverAllScreens = { baseScreenPath ->
+                def processScreenWithSubscreens 
-                    def allScreens = [] as Set<String>
+                processScreenWithSubscreens = { screenPath, parentScreenPath = null, processedScreens = null, toolsAccumulator = null ->
-                    def screensToProcess = [baseScreenPath]
+                    ec.logger.info("MCP Screen Discovery: Processing screen ${screenPath} (parent: ${parentScreenPath})")
-                    ec.logger.info("MCP Screen Discovery: Starting discovery for base ${baseScreenPath}")
+                    // Initialize processedScreens and toolsAccumulator if null
+                    if (processedScreens == null) processedScreens = [] as Set<String>
+                    if (toolsAccumulator == null) toolsAccumulator = []
-                    while (screensToProcess) {
+                    if (processedScreens.contains(screenPath)) {
-                        def currentScreenPath = screensToProcess.remove(0)
+                        ec.logger.info("MCP Screen Discovery: Already processed ${screenPath}, skipping")
+                        return
-                        ec.logger.info("MCP Screen Discovery: Processing screen ${currentScreenPath}")
-                        if (allScreens.contains(currentScreenPath)) {
-                            ec.logger.info("MCP Screen Discovery: Already processed ${currentScreenPath}, skipping")
-                            continue
-                        }
-                        allScreens.add(currentScreenPath)
-                        try {
-                            ec.logger.info("MCP Screen Discovery: Getting screen info for ${currentScreenPath}")
-                            // Fix: Use getScreenInfoList and properly access the ScreenInfo object
-                            def screenInfoList = ec.screen.getScreenInfoList(currentScreenPath, 1)
-                            if (screenInfoList && screenInfoList.size() > 0) {
-                                def screenInfo = screenInfoList.first()
-                                ec.logger.info("MCP Screen Discovery: Screen info for ${currentScreenPath}: subscreens=${screenInfo?.subscreenInfoByName?.size() ?: 0}")
-                                // Fix: Use the correct property name 'subscreenInfoByName' (not 'subScreenInfoByName')
-                                if (screenInfo?.subscreenInfoByName) {
-                                    ec.logger.info("MCP Screen Discovery: Found subscreens map: ${screenInfo.subscreenInfoByName.keySet()}")
-                                    for (subScreenEntry in screenInfo.subscreenInfoByName) {
-                                        def subScreenInfo = subScreenEntry.value
-                                        def subScreenPath = subScreenInfo?.sd?.location
-                                        ec.logger.info("MCP Screen Discovery: Found subscreen ${subScreenPath} for ${currentScreenPath}")
-                                        if (subScreenPath && !allScreens.contains(subScreenPath)) {
-                                            screensToProcess.add(subScreenPath)
-                                            ec.logger.info("MCP Screen Discovery: Added ${subScreenPath} to processing queue")
-                                        }
-                                    }
-                                } else {
-                                    ec.logger.info("MCP Screen Discovery: No subscreens found for ${currentScreenPath}")
-                                }
-                            } else {
-                                ec.logger.info("MCP Screen Discovery: No screen info returned for ${currentScreenPath}")
-                            }
-                        } catch (Exception e) {
-                            ec.logger.info("MCP Screen Discovery: Could not get subscreens for ${currentScreenPath}: ${e.message}")
-                            ec.logger.error("MCP Screen Discovery: Error details:", e)
-                        }
                    }
-                    return allScreens
+                    processedScreens.add(screenPath)
-                }
-                // Discover all screens recursively starting from accessible screens
-                def allAccessibleScreens = [] as Set<String>
-                ec.logger.info("MCP Screen Discovery: Starting recursive discovery from ${accessibleScreens.size()} base screens")
-                for (screenPath in accessibleScreens) {
-                    def discoveredScreens = discoverAllScreens(screenPath)
-                    ec.logger.info("MCP Screen Discovery: Found ${discoveredScreens.size()} screens from base ${screenPath}")
-                    allAccessibleScreens.addAll(discoveredScreens)
-                }
-                ec.logger.info("MCP Screen Discovery: Recursive discovery found ${allAccessibleScreens.size()} total screens")
-                // Use recursively discovered screens instead of hardcoded list
-                for (screenPath in allAccessibleScreens) {
                    try {
-                        //ec.logger.info("MCP Screen Discovery: Processing screen ${screenPath}")
+                        // Skip problematic patterns early
-                        // For MCP, include all accessible screens - LLMs can decide what's useful
-                        // Skip only obviously problematic patterns
                        if (screenPath.contains("/error/") || screenPath.contains("/system/")) {
                            ec.logger.info("MCP Screen Discovery: Skipping system screen ${screenPath}")
-                            continue
+                            return
                        }
-                        // Try to get screen definition, but don't require it for MCP
+                        // Determine if this is a subscreen
+                        def isSubscreen = parentScreenPath != null
+                        // Try to get screen definition
                        def screenDefinition = null
                        def title = screenPath.split("/")[-1]
                        def description = "Moqui screen: ${screenPath}"
                        try {
                            screenDefinition = ec.screen.getScreenDefinition(screenPath)
-                            // Screen XML doesn't have description elements, so use screen path as description
-                            // We could potentially use default-menu-title attribute if available
                            if (screenDefinition?.screenNode?.attribute('default-menu-title')) {
                                title = screenDefinition.screenNode.attribute('default-menu-title')
                                description = "Moqui screen: ${screenPath} (${title})"
                            }
                        } catch (Exception e) {
                            ec.logger.info("MCP Screen Discovery: No screen definition for ${screenPath}, using basic info")
-                            // Continue anyway - the screen might still be useful for MCP
                        }
                        // Get screen parameters from transitions
@@ -1155,19 +1090,19 @@ try {
                            if (screenInfo?.transitionInfoByName) {
                                for (transitionEntry in screenInfo.transitionInfoByName) {
                                    def transitionInfo = transitionEntry.value
-                                    // Add path parameters
+                                    if (transitionInfo?.ti) {
-                                    transitionInfo.ti?.getPathParameterList()?.each { param ->
+                                        transitionInfo.ti.getPathParameterList()?.each { param ->
-                                        parameters[param] = [
+                                            parameters[param] = [
-                                            type: "string",
+                                                type: "string",
-                                            description: "Path parameter for transition: ${param}"
+                                                description: "Path parameter for transition: ${param}"
-                                        ]
+                                            ]
-                                    }
+                                        }
-                                    // Add request parameters
+                                        transitionInfo.ti.getRequestParameterList()?.each { param ->
-                                    transitionInfo.ti?.getRequestParameterList()?.each { param ->
+                                            parameters[param.name] = [
-                                        parameters[param.name] = [
+                                                type: "string",
-                                            type: "string",
+                                                description: "Request parameter: ${param.name}"
-                                            description: "Request parameter: ${param.name}"
+                                            ]
-                                        ]
+                                        }
                                    }
                                }
                            }
@@ -1175,14 +1110,95 @@ try {
                            ec.logger.debug("Could not extract parameters from screen ${screenPath}: ${e.message}")
                        }
-                        ec.logger.info("MCP Screen Discovery: Adding accessible screen ${screenPath}")
+                        // Create tool with proper naming
-                        tools << createScreenTool(screenPath, title, description, parameters)
+                        def toolName
+                        if (isSubscreen && parentScreenPath) {
+                            toolName = screenPathToToolNameWithSubscreens(screenPath, parentScreenPath)
+                            ec.logger.info("MCP Screen Discovery: Creating subscreen tool ${toolName} for ${screenPath} (parent: ${parentScreenPath})")
+                        } else {
+                            toolName = screenPathToToolName(screenPath)
+                            ec.logger.info("MCP Screen Discovery: Creating main screen tool ${toolName} for ${screenPath}")
+                        }
+                        def tool = [
+                            name: toolName,
+                            title: title,
+                            description: "Moqui screen: ${screenPath}",
+                            inputSchema: [
+                                type: "object",
+                                properties: parameters,
+                                required: []
+                            ]
+                        ]
+                        ec.logger.info("MCP Screen Discovery: Adding accessible screen tool ${toolName} for ${screenPath}")
+                        toolsAccumulator << tool
+                        // Recursively process subscreens
+                        try {
+                            def screenInfoList = ec.screen.getScreenInfoList(screenPath, 1)
+                            def screenInfo = screenInfoList?.first()
+                            ec.logger.info("MCP Screen Discovery: SCREENINFO ${screenInfo}")
+                            if (screenInfo?.subscreenInfoByName) {
+                                ec.logger.info("MCP Screen Discovery: Found ${screenInfo.subscreenInfoByName.size()} subscreens for ${screenPath}")
+                                for (subScreenEntry in screenInfo.subscreenInfoByName) {
+                                    ec.logger.info("MCP Screen Discovery: Process subscreen ${subScreenEntry.key}")
+                                    def subScreenInfo = subScreenEntry.value
+                                    def subScreenPathList = subScreenInfo?.screenPath
+                                    ec.logger.info("MCP Screen Discovery: Processing subscreen entry - key: ${subScreenEntry.key}, location: ${subScreenPathList}")
+                                    // Ensure subScreenPathList is a flat list of strings
+                                    def flatPathList = []
+                                    if (subScreenPathList) {
+                                        if (subScreenPathList instanceof Collection) {
+                                            for (path in subScreenPathList) {
+                                                if (path instanceof Collection) {
+                                                    flatPathList.addAll(path)
+                                                } else {
+                                                    flatPathList.add(path)
+                                                }
+                                            }
+                                        } else {
+                                            flatPathList.add(subScreenPathList)
+                                        }
+                                    }
+                                        for (subScreenPath in flatPathList) {
+                                            if (subScreenPath && !processedScreens.contains(subScreenPath)) {
+                                                ec.logger.info("MCP Screen Discovery: Processing subscreen path: ${subScreenPath}")
+                                                processScreenWithSubscreens(subScreenPath, screenPath, processedScreens, toolsAccumulator)
+                                        } else if (!subScreenPath) {
+                                            ec.logger.info("MCP Screen Discovery: Subscreen entry ${subScreenEntry.key} has no location, skipping")
+                                        } else if (processedScreens.contains(subScreenPath)) {
+                                            ec.logger.info("MCP Screen Discovery: Subscreen ${subScreenPath} already processed, skipping")
+                                        }
+                                    }
+                                }
+                            } else {
+                                ec.logger.info("MCP Screen Discovery: No subscreens found for ${screenPath}")
+                            }
+                        } catch (Exception e) {
+                            ec.logger.info("MCP Screen Discovery: Could not get subscreens for ${screenPath}: ${e.message}")
+                            ec.logger.error("MCP Screen Discovery: Subscreen discovery error details:", e)
+                        }
                    } catch (Exception e) {
                        ec.logger.warn("Error processing screen ${screenPath}: ${e.message}")
                    }
                }
+                // Process all accessible screens recursively and create tools directly
+                def processedScreens = [] as Set<String>
+                ec.logger.info("MCP Screen Discovery: Starting recursive processing from ${accessibleScreens.size()} base screens")
+                for (screenPath in accessibleScreens) {
+                    ec.logger.info("MCP Screen Discovery: SCREEN PATH ${screenPath}")
+                    processScreenWithSubscreens(screenPath, null, processedScreens, tools)
+                }
+                ec.logger.info("MCP Screen Discovery: Recursive processing found ${tools.size()} total tools")
+                // Note: All screens have already been processed by processScreenWithSubscreens above
+                // The recursive approach handles both parent screens and their subscreens in a single pass
+                // No need for additional processing here
                ec.logger.info("MCP Screen Discovery: Created ${tools.size()} screen tools for user ${originalUsername}")
                result.tools = tools
@@ -1358,6 +1374,7 @@ try {
            <parameter name="parameters" type="Map"><description>Parameters to pass to the screen</description></parameter>
            <parameter name="renderMode" default="html"><description>Render mode: text, html, xml, vuet, qvt</description></parameter>
            <parameter name="sessionId"><description>Session ID for user context restoration</description></parameter>
+            <parameter name="subscreenName"><description>Optional subscreen name for dot notation paths</description></parameter>
        </in-parameters>
        <out-parameters>
            <parameter name="result" type="Map"/>
@@ -1397,12 +1414,12 @@ def startTime = System.currentTimeMillis()
        def targetScreenDef = null
        def isStandalone = false
+            def pathAfterComponent = screenPath.substring(12).replace('.xml','') // Remove "component://"
+            def pathParts = pathAfterComponent.split("/")
                // If the screen path is already a full component:// path, we need to handle it differently
        if (screenPath.startsWith("component://")) {
            // For component:// paths, we need to use the component's root screen, not webroot
            // Extract the component name and use its root screen
-            def pathAfterComponent = screenPath.substring(12) // Remove "component://"
-            def pathParts = pathAfterComponent.split("/")
            if (pathParts.length >= 2) {
                def componentName = pathParts[0]
                def remainingPath = pathParts[1..-1].join("/")
@@ -1470,8 +1487,8 @@ def startTime = System.currentTimeMillis()
                            def componentRootScreen = null
                            def possibleRootScreens = [
                                "${componentName}.xml",
-                                "${componentName}Root.xml", 
+                                "${componentName}Admin.xml",
-                                "${componentName}Admin.xml"
+                                "${componentName}Root.xml"
                            ]
                            for (rootScreenName in possibleRootScreens) {
@@ -1519,71 +1536,57 @@ def startTime = System.currentTimeMillis()
            }
        }
-        // User context should already be correct from MCP servlet restoration
+        // Handle subscreen if specified
-        // CustomScreenTestImpl will capture current user context automatically
+        if (subscreenName) {
-        ec.logger.info("MCP Screen Execution: Current user context - userId: ${ec.user.userId}, username: ${ec.user.username}")
+            ec.logger.info("MCP Screen Execution: Handling subscreen ${subscreenName} for parent ${screenPath}")
-        // Regular screen rendering with current user context - use our custom ScreenTestImpl
-        def screenTest = new org.moqui.mcp.CustomScreenTestImpl(ec.ecfi)
-            .rootScreen(rootScreen)
-            .renderMode(renderMode ? renderMode : "html")
-            .auth(ec.user.username) // Propagate current user to the test renderer
-        ec.logger.info("MCP Screen Execution: ScreenTest object created: ${screenTest?.getClass()?.getSimpleName()}")
-                if (screenTest) {
+            // For subscreens, we need to modify the render path to include the subscreen
-                    def renderParams = parameters ?: [:]
+            // The pathParts array already contains the full path, so we need to add the subscreen name
+            def subscreenPathParts = pathParts + subscreenName.split('_')
-                    // Add current user info to render context to maintain authentication
+            ec.logger.info("MCP Screen Execution: Full subscreen path parts: ${subscreenPathParts}")
-                    renderParams.userId = ec.user.userId
-                    renderParams.username = ec.user.username
+            // User context should already be correct from MCP servlet restoration
+            // CustomScreenTestImpl will capture current user context automatically
-                    // Set user context in ScreenTest to maintain authentication
+            ec.logger.info("MCP Screen Execution: Current user context - userId: ${ec.user.userId}, username: ${ec.user.username}")
-                    // Note: ScreenTestImpl may not have direct userAccountId property, 
-                    // the user context should be inherited from the current ExecutionContext
+            // Regular screen rendering with current user context - use our custom ScreenTestImpl
-                    ec.logger.info("MCP Screen Execution: Current user context - userId: ${ec.user.userId}, username: ${ec.user.username}")
+            def screenTest = new org.moqui.mcp.CustomScreenTestImpl(ec.ecfi)
+                .rootScreen(rootScreen)
-                    // Regular screen rendering with timeout
+                .renderMode(renderMode ? renderMode : "html")
-                    def future = java.util.concurrent.Executors.newSingleThreadExecutor().submit({
+                .auth(ec.user.username) // Propagate current user to the test renderer
-                        return screenTest.render(testScreenPath, renderParams, null)
-                    } as java.util.concurrent.Callable)
+            ec.logger.info("MCP Screen Execution: ScreenTest object created for subscreen: ${screenTest?.getClass()?.getSimpleName()}")
-                    try {
+            if (screenTest) {
-                        def testRender = future.get(30, java.util.concurrent.TimeUnit.SECONDS)  // 30 second timeout
+                def renderParams = parameters ?: [:]
-                        output = testRender.output
-                        def outputLength = output?.length() ?: 0
+                // Add current user info to render context to maintain authentication
+                renderParams.userId = ec.user.userId
-                        // SIZE PROTECTION: Check response size before returning
+                renderParams.username = ec.user.username
-                        def maxResponseSize = 1024 * 1024  // 1MB limit
-                        if (outputLength > maxResponseSize) {
+                // Set user context in ScreenTest to maintain authentication
-                            isError = true
+                // Note: ScreenTestImpl may not have direct userAccountId property, 
-                            ec.logger.warn("MCP Screen Execution: Response too large for ${screenPath}: ${outputLength} bytes (limit: ${maxResponseSize} bytes)")
+                // the user context should be inherited from the current ExecutionContext
+                ec.logger.info("MCP Screen Execution: Current user context - userId: ${ec.user.userId}, username: ${ec.user.username}")
-                            // Create truncated response with clear indication
+                ec.logger.info("SUBSCREEN PATH PARTS ${subscreenPathParts}")
-                            def truncatedOutput = output.substring(0, Math.min(maxResponseSize / 2, outputLength))
-                            output = """SCREEN RESPONSE TRUNCATED
+            // Regular screen rendering with timeout for subscreen
-The screen '${screenPath}' generated a response that is too large for MCP processing:
+            try {
- Original size: ${outputLength} bytes
+                ec.logger.info("TESTRENDER ${subscreenPathParts} ${renderParams}")
- Size limit: ${maxResponseSize} bytes
+                // For subscreens, the path should be relative to the parent screen that's already set as root
- Truncated to: ${truncatedOutput.length()} bytes
+                // Since we're using the parent screen as root, we only need the subscreen name part
+                def testRender = screenTest.render(subscreenName.replaceAll('_','/'), renderParams, "POST")
-TRUNCATED CONTENT:
+                output = testRender.getOutput()
-${truncatedOutput}
+                def outputLength = output?.length() ?: 0
+                ec.logger.info("MCP Screen Execution: Successfully rendered screen ${screenPath}, output length: ${output?.length() ?: 0}")
-[Response truncated due to size limits. Consider using more specific screen parameters or limiting data ranges.]"""
+            } catch (java.util.concurrent.TimeoutException e) {
-                        }
+                throw new Exception("Screen rendering timed out after 30 seconds for ${screenPath}")
+            }
-                        ec.logger.info("MCP Screen Execution: Successfully rendered screen ${screenPath}, output length: ${output?.length() ?: 0}")
+        } else {
-                    } catch (java.util.concurrent.TimeoutException e) {
+            throw new Exception("ScreenTest object is null")
-                        future.cancel(true)
+        }
-                        throw new Exception("Screen rendering timed out after 30 seconds for ${screenPath}")
+        }
-                    } finally {
-                        future.cancel(true)
-                    }
-                } else {
-                    throw new Exception("ScreenTest object is null")
-                }
    } catch (Exception e) {
        isError = true
        ec.logger.warn("MCP Screen Execution: Could not render screen ${screenPath}, exposing error details: ${e.message}")
@@ -1711,7 +1714,7 @@ ${truncatedOutput}
    // Return just the rendered screen content for MCP wrapper to handle
    result = [
-        type: "text",
+        type: "html",
        text: processedOutput,
        screenPath: screenPath,
        screenUrl: screenUrl,
--- a/src/main/groovy/org/moqui/mcp/CustomScreenTestImpl.groovy
View file @e18de6c
+++ b/src/main/groovy/org/moqui/mcp/CustomScreenTestImpl.groovy
View file @e18de6c
@@ -150,10 +150,12 @@ class CustomScreenTestImpl implements McpScreenTest {
    void renderAll(List<String> screenPathList, Map<String, Object> parameters, String requestMethod) {
        // NOTE: using single thread for now, doesn't actually make a lot of difference in overall test run time
        int threads = 1
+        def output
        if (threads == 1) {
            for (String screenPath in screenPathList) {
                McpScreenTestRender str = render(screenPath, parameters, requestMethod)
-                logger.info("Rendered ${screenPath} in ${str.getRenderTime()}ms, ${str.getOutput()?.length()} characters")
+                output = str.getOutput()
+                logger.info("Rendered ${screenPath} in ${str.getRenderTime()}ms, ${output?.length()} characters")
            }
        } else {
            ExecutionContextImpl eci = ecfi.getEci()
@@ -264,8 +266,26 @@ class CustomScreenTestImpl implements McpScreenTest {
            long startTime = System.currentTimeMillis()
            // parse the screenPath
-            ArrayList<String> screenPathList = ScreenUrlInfo.parseSubScreenPath(csti.rootScreenDef, csti.baseScreenDef,
+            def screenPathList
-                    csti.baseScreenPathList, stri.screenPath, stri.parameters, csti.sfi)
+            // Special handling for non-webroot root screens with subscreens
+            if (csti.rootScreenLocation != null && !csti.rootScreenLocation.contains("webroot.xml") && stri.screenPath.contains('/')) {
+                // For non-webroot roots with subscreens, build path list directly
+                // rootScreenDef is the parent screen, screenPath is the subscreen path
+                screenPathList = new ArrayList<>()
+                // Add root screen path (already a full component:// path)
+                screenPathList.add(csti.rootScreenDef.location)
+                // Add subscreen path segments
+                String[] pathSegments = stri.screenPath.split('/')
+                for (String segment in pathSegments) {
+                    if (segment && segment.trim().length() > 0) {
+                        screenPathList.add(segment)
+                    }
+                }
+                logger.info("Custom screen path parsing for non-webroot root: ${screenPathList}")
+            } else {
+                screenPathList = ScreenUrlInfo.parseSubScreenPath(csti.rootScreenDef, csti.baseScreenDef,
+                        csti.baseScreenPathList, stri.screenPath, stri.parameters, csti.sfi)
+            }
            if (screenPathList == null) throw new BaseArtifactException("Could not find screen path ${stri.screenPath} under base screen ${csti.baseScreenDef.location}")
            // push the context
@@ -292,7 +312,7 @@ class CustomScreenTestImpl implements McpScreenTest {
            }
            // set the screenPath
-            screenRender.screenPath(screenPathList)
+            screenRender.screenPath(screenPathList as java.util.List<String>)
            // do the render
            try {
--- a/src/main/groovy/org/moqui/mcp/EnhancedMcpServlet.groovy
View file @e18de6c
+++ b/src/main/groovy/org/moqui/mcp/EnhancedMcpServlet.groovy
View file @e18de6c
@@ -862,8 +862,13 @@ logger.info("Handling Enhanced SSE connection from ${request.remoteAddr}")
                logger.error("Enhanced MCP service ${serviceName} returned null result")
                return [error: "Service returned null result"]
            }
-            // Service framework returns result in 'result' field, but also might return the result directly
+            // Service framework returns result in 'result' field when out-parameters are used
-            return result.result ?: result ?: [error: "Service returned invalid result"]
+            // Unwrap the Moqui service result to avoid double nesting in JSON-RPC response
+            if (result?.containsKey('result')) {
+                return result.result ?: [error: "Service returned empty result"]
+            } else {
+                return result ?: [error: "Service returned null result"]
+            }
        } catch (Exception e) {
            logger.error("Error calling Enhanced MCP service ${serviceName}", e)
            return [error: e.message]