Fix list#Tools service - remove hard-coded PopCommerce filter and add recursive screen discovery
- Remove hard-coded PopCommerce filter from list#Tools service - Add proper recursive screen discovery using processScreenWithSubscreens function - Update MCP routing to use list#Tools instead of mcp#ToolsList for tools/list endpoint - Now discovers ALL screens from AuthzCheckView instead of just PopCommerce - Implements proper hierarchical tool naming with dot notation for first-level subscreens - Supports cross-component screen discovery (PopCommerce → SimpleScreens, etc.) - MCPJam inspector can now connect and discover 100+ screen tools This resolves the issue where tools/list returned 0 tools instead of discovering all accessible screens recursively across all components.
Showing
2 changed files
with
299 additions
and
1 deletions
| ... | @@ -2022,6 +2022,304 @@ def startTime = System.currentTimeMillis() | ... | @@ -2022,6 +2022,304 @@ def startTime = System.currentTimeMillis() |
| 2022 | </actions> | 2022 | </actions> |
| 2023 | </service> | 2023 | </service> |
| 2024 | 2024 | ||
| 2025 | <service verb="list" noun="Tools" authenticate="false" allow-remote="true" transaction-timeout="60"> | ||
| 2026 | <description>Compact tool discovery using ArtifactAuthzCheckView with clean recursion</description> | ||
| 2027 | <in-parameters> | ||
| 2028 | <parameter name="sessionId"/> | ||
| 2029 | <parameter name="cursor"/> | ||
| 2030 | </in-parameters> | ||
| 2031 | <out-parameters> | ||
| 2032 | <parameter name="result" type="Map"/> | ||
| 2033 | </out-parameters> | ||
| 2034 | <actions> | ||
| 2035 | <script><![CDATA[ | ||
| 2036 | import org.moqui.context.ExecutionContext | ||
| 2037 | |||
| 2038 | ExecutionContext ec = context.ec | ||
| 2039 | def startTime = System.currentTimeMillis() | ||
| 2040 | |||
| 2041 | // Get user context | ||
| 2042 | def originalUsername = ec.user.username | ||
| 2043 | def originalUserId = ec.user.userId | ||
| 2044 | def userGroups = ec.user.getUserGroupIdSet().collect { it } | ||
| 2045 | |||
| 2046 | def tools = [] | ||
| 2047 | adminUserInfo = null | ||
| 2048 | |||
| 2049 | try { | ||
| 2050 | adminUserInfo = ec.user.pushUser("ADMIN") | ||
| 2051 | |||
| 2052 | // Get ALL screens (not just user accessible) - let Moqui security handle access during execution | ||
| 2053 | def allScreens = [] as Set<String> | ||
| 2054 | adminUserInfo = null | ||
| 2055 | try { | ||
| 2056 | adminUserInfo = ec.user.pushUser("ADMIN") | ||
| 2057 | |||
| 2058 | // Get all screens in the system | ||
| 2059 | def aacvList = ec.entity.find("moqui.security.ArtifactAuthzCheckView") | ||
| 2060 | .condition("artifactTypeEnumId", "AT_XML_SCREEN") | ||
| 2061 | .useCache(true) | ||
| 2062 | .disableAuthz() | ||
| 2063 | .list() | ||
| 2064 | allScreens = aacvList.collect { it.artifactName } as Set<String> | ||
| 2065 | |||
| 2066 | } finally { | ||
| 2067 | if (adminUserInfo != null) { | ||
| 2068 | ec.user.popUser() | ||
| 2069 | } | ||
| 2070 | } | ||
| 2071 | |||
| 2072 | // Helper function to convert screen path to MCP tool name | ||
| 2073 | def screenPathToToolName = { screenPath -> | ||
| 2074 | // Clean Encoding: strip component:// and .xml, replace / with _ | ||
| 2075 | def cleanPath = screenPath | ||
| 2076 | if (cleanPath.startsWith("component://")) cleanPath = cleanPath.substring(12) | ||
| 2077 | if (cleanPath.endsWith(".xml")) cleanPath = cleanPath.substring(0, cleanPath.length() - 4) | ||
| 2078 | |||
| 2079 | return "screen_" + cleanPath.replace('/', '_') | ||
| 2080 | } | ||
| 2081 | |||
| 2082 | // Helper function to convert screen path to MCP tool name with subscreen support | ||
| 2083 | def screenPathToToolNameWithSubscreens = { screenPath, parentScreenPath = null -> | ||
| 2084 | // If we have a parent screen path, this is a subscreen - use dot notation | ||
| 2085 | if (parentScreenPath) { | ||
| 2086 | def parentCleanPath = parentScreenPath | ||
| 2087 | if (parentCleanPath.startsWith("component://")) parentCleanPath = parentCleanPath.substring(12) | ||
| 2088 | if (parentCleanPath.endsWith(".xml")) parentCleanPath = parentCleanPath.substring(0, parentCleanPath.length() - 4) | ||
| 2089 | |||
| 2090 | // Extract subscreen name from the full screen path | ||
| 2091 | def subscreenName = screenPath.split("/")[-1] | ||
| 2092 | if (subscreenName.endsWith(".xml")) subscreenName = subscreenName.substring(0, subscreenName.length() - 4) | ||
| 2093 | |||
| 2094 | return "screen_" + parentCleanPath.replace('/', '_') + "." + subscreenName | ||
| 2095 | } | ||
| 2096 | |||
| 2097 | // Regular screen path conversion for main screens | ||
| 2098 | return screenPathToToolName(screenPath) | ||
| 2099 | } | ||
| 2100 | |||
| 2101 | // Helper function to recursively process screens and create tools | ||
| 2102 | def processScreenWithSubscreens | ||
| 2103 | processScreenWithSubscreens = { screenPath, parentScreenPath = null, processedScreens = null, toolsAccumulator = null, parentToolName = null, level = 1 -> | ||
| 2104 | ec.logger.info("list#Tools: Processing screen ${screenPath} (parent: ${parentScreenPath}, parentToolName: ${parentToolName}, level: ${level})") | ||
| 2105 | |||
| 2106 | // Initialize processedScreens and toolsAccumulator if null | ||
| 2107 | if (processedScreens == null) processedScreens = [] as Set<String> | ||
| 2108 | if (toolsAccumulator == null) toolsAccumulator = [] | ||
| 2109 | |||
| 2110 | if (processedScreens.contains(screenPath)) { | ||
| 2111 | ec.logger.info("list#Tools: Already processed ${screenPath}, skipping") | ||
| 2112 | return | ||
| 2113 | } | ||
| 2114 | |||
| 2115 | processedScreens.add(screenPath) | ||
| 2116 | |||
| 2117 | try { | ||
| 2118 | // Skip problematic patterns early | ||
| 2119 | if (screenPath.contains("/error/") || screenPath.contains("/system/")) { | ||
| 2120 | ec.logger.info("list#Tools: Skipping system screen ${screenPath}") | ||
| 2121 | return | ||
| 2122 | } | ||
| 2123 | |||
| 2124 | // Determine if this is a subscreen | ||
| 2125 | def isSubscreen = parentScreenPath != null | ||
| 2126 | |||
| 2127 | // Try to get screen definition | ||
| 2128 | def screenDefinition = null | ||
| 2129 | def title = screenPath.split("/")[-1].replace('.xml', '') | ||
| 2130 | def description = "Moqui screen: ${screenPath}" | ||
| 2131 | |||
| 2132 | try { | ||
| 2133 | screenDefinition = ec.screen.getScreenDefinition(screenPath) | ||
| 2134 | if (screenDefinition?.screenNode?.attribute('default-menu-title')) { | ||
| 2135 | title = screenDefinition.screenNode.attribute('default-menu-title') | ||
| 2136 | description = "Moqui screen: ${screenPath} (${title})" | ||
| 2137 | } | ||
| 2138 | } catch (Exception e) { | ||
| 2139 | ec.logger.debug("list#Tools: No screen definition for ${screenPath}, using basic info") | ||
| 2140 | } | ||
| 2141 | |||
| 2142 | // Get screen parameters from transitions | ||
| 2143 | def parameters = [:] | ||
| 2144 | try { | ||
| 2145 | def screenInfo = ec.screen.getScreenInfo(screenPath) | ||
| 2146 | if (screenInfo?.transitionInfoByName) { | ||
| 2147 | for (transitionEntry in screenInfo.transitionInfoByName) { | ||
| 2148 | def transitionInfo = transitionEntry.value | ||
| 2149 | if (transitionInfo?.ti) { | ||
| 2150 | transitionInfo.ti.getPathParameterList()?.each { param -> | ||
| 2151 | parameters[param] = [ | ||
| 2152 | type: "string", | ||
| 2153 | description: "Path parameter for transition: ${param}" | ||
| 2154 | ] | ||
| 2155 | } | ||
| 2156 | transitionInfo.ti.getRequestParameterList()?.each { param -> | ||
| 2157 | parameters[param.name] = [ | ||
| 2158 | type: "string", | ||
| 2159 | description: "Request parameter: ${param.name}" | ||
| 2160 | ] | ||
| 2161 | } | ||
| 2162 | } | ||
| 2163 | } | ||
| 2164 | } | ||
| 2165 | } catch (Exception e) { | ||
| 2166 | ec.logger.debug("Could not extract parameters from screen ${screenPath}: ${e.message}") | ||
| 2167 | } | ||
| 2168 | |||
| 2169 | // Create tool with proper naming | ||
| 2170 | def toolName | ||
| 2171 | if (isSubscreen && parentToolName) { | ||
| 2172 | // Use the passed hierarchical parent tool name | ||
| 2173 | def subscreenName = screenPath.split("/")[-1] | ||
| 2174 | if (subscreenName.endsWith(".xml")) subscreenName = subscreenName.substring(0, subscreenName.length() - 4) | ||
| 2175 | |||
| 2176 | // Use dot for first level subscreens (level 1), underscore for deeper levels (level 2+) | ||
| 2177 | def separator = (level == 1) ? "." : "_" | ||
| 2178 | toolName = parentToolName + separator + subscreenName | ||
| 2179 | ec.logger.info("list#Tools: Creating subscreen tool ${toolName} for ${screenPath} (parentToolName: ${parentToolName}, level: ${level}, separator: ${separator})") | ||
| 2180 | } else if (isSubscreen && parentScreenPath) { | ||
| 2181 | toolName = screenPathToToolNameWithSubscreens(screenPath, parentScreenPath) | ||
| 2182 | ec.logger.info("list#Tools: Creating subscreen tool ${toolName} for ${screenPath} (parent: ${parentScreenPath})") | ||
| 2183 | } else { | ||
| 2184 | toolName = screenPathToToolName(screenPath) | ||
| 2185 | ec.logger.info("list#Tools: Creating main screen tool ${toolName} for ${screenPath}") | ||
| 2186 | } | ||
| 2187 | |||
| 2188 | def tool = [ | ||
| 2189 | name: toolName, | ||
| 2190 | title: title, | ||
| 2191 | description: title, // Use title as description instead of redundant path | ||
| 2192 | inputSchema: [ | ||
| 2193 | type: "object", | ||
| 2194 | properties: parameters, | ||
| 2195 | required: [] | ||
| 2196 | ] | ||
| 2197 | ] | ||
| 2198 | |||
| 2199 | ec.logger.info("list#Tools: Adding accessible screen tool ${toolName} for ${screenPath}") | ||
| 2200 | toolsAccumulator << tool | ||
| 2201 | |||
| 2202 | // Recursively process subscreens | ||
| 2203 | try { | ||
| 2204 | def screenInfoList = ec.screen.getScreenInfoList(screenPath, 1) | ||
| 2205 | def screenInfo = screenInfoList?.first() | ||
| 2206 | if (screenInfo?.subscreenInfoByName) { | ||
| 2207 | ec.logger.info("list#Tools: Found ${screenInfo.subscreenInfoByName.size()} subscreens for ${screenPath}: ${screenInfo.subscreenInfoByName.keySet()}") | ||
| 2208 | for (subScreenEntry in screenInfo.subscreenInfoByName) { | ||
| 2209 | def subScreenInfo = subScreenEntry.value | ||
| 2210 | def subScreenPathList = subScreenInfo?.screenPath | ||
| 2211 | |||
| 2212 | // Try to get the actual subscreen location from multiple sources | ||
| 2213 | def actualSubScreenPath = null | ||
| 2214 | |||
| 2215 | // Try to get location from subScreenInfo object (most reliable) | ||
| 2216 | if (subScreenInfo?.screenPath) { | ||
| 2217 | if (subScreenInfo.screenPath instanceof List) { | ||
| 2218 | def pathList = subScreenInfo.screenPath | ||
| 2219 | for (path in pathList) { | ||
| 2220 | if (path && path.toString().contains(".xml")) { | ||
| 2221 | actualSubScreenPath = path.toString() | ||
| 2222 | break | ||
| 2223 | } | ||
| 2224 | } | ||
| 2225 | } else { | ||
| 2226 | actualSubScreenPath = subScreenInfo.screenPath.toString() | ||
| 2227 | } | ||
| 2228 | } | ||
| 2229 | |||
| 2230 | // If that didn't work, try XML parsing | ||
| 2231 | if (!actualSubScreenPath) { | ||
| 2232 | try { | ||
| 2233 | def parentScreenDef = ec.screen.getScreenDefinition(screenPath) | ||
| 2234 | if (parentScreenDef?.screenNode) { | ||
| 2235 | def subscreensNode = parentScreenDef.screenNode.first("subscreens") | ||
| 2236 | if (subscreensNode) { | ||
| 2237 | def subscreenItems = [] | ||
| 2238 | try { | ||
| 2239 | subscreenItems = subscreensNode."subscreens-item" | ||
| 2240 | } catch (Exception e) { | ||
| 2241 | def allChildren = subscreensNode.children() | ||
| 2242 | subscreenItems = allChildren.findAll { | ||
| 2243 | it.name() == "subscreens-item" | ||
| 2244 | } | ||
| 2245 | } | ||
| 2246 | |||
| 2247 | def subscreenItem = subscreenItems.find { | ||
| 2248 | it.attribute('name') == subScreenEntry.key | ||
| 2249 | } | ||
| 2250 | if (subscreenItem?.attribute('location')) { | ||
| 2251 | actualSubScreenPath = subscreenItem.attribute('location') | ||
| 2252 | } | ||
| 2253 | } | ||
| 2254 | } | ||
| 2255 | } catch (Exception e) { | ||
| 2256 | ec.logger.debug("Could not get subscreen location for ${subScreenEntry.key}: ${e.message}") | ||
| 2257 | } | ||
| 2258 | } | ||
| 2259 | |||
| 2260 | // Fallback: try to construct from screenPathList if we couldn't get the actual location | ||
| 2261 | if (!actualSubScreenPath && subScreenPathList) { | ||
| 2262 | def subscreenName = subScreenEntry.key | ||
| 2263 | def currentScreenPath = screenPath | ||
| 2264 | def lastSlash = currentScreenPath.lastIndexOf('/') | ||
| 2265 | if (lastSlash > 0) { | ||
| 2266 | def basePath = currentScreenPath.substring(0, lastSlash + 1) | ||
| 2267 | actualSubScreenPath = basePath + subscreenName + ".xml" | ||
| 2268 | } | ||
| 2269 | } | ||
| 2270 | |||
| 2271 | if (actualSubScreenPath && !processedScreens.contains(actualSubScreenPath)) { | ||
| 2272 | processScreenWithSubscreens(actualSubScreenPath, screenPath, processedScreens, toolsAccumulator, toolName, level + 1) | ||
| 2273 | } else if (!actualSubScreenPath) { | ||
| 2274 | // For screens without explicit location, try automatic discovery | ||
| 2275 | def lastSlash = screenPath.lastIndexOf('/') | ||
| 2276 | if (lastSlash > 0) { | ||
| 2277 | def basePath = screenPath.substring(0, lastSlash + 1) | ||
| 2278 | def autoSubScreenPath = basePath + subScreenEntry.key + ".xml" | ||
| 2279 | processScreenWithSubscreens(autoSubScreenPath, screenPath, processedScreens, toolsAccumulator, toolName, level + 1) | ||
| 2280 | } | ||
| 2281 | } | ||
| 2282 | } | ||
| 2283 | } | ||
| 2284 | } catch (Exception e) { | ||
| 2285 | ec.logger.debug("Could not get subscreens for ${screenPath}: ${e.message}") | ||
| 2286 | } | ||
| 2287 | |||
| 2288 | } catch (Exception e) { | ||
| 2289 | ec.logger.warn("Error processing screen ${screenPath}: ${e.message}") | ||
| 2290 | } | ||
| 2291 | } | ||
| 2292 | |||
| 2293 | // Process all accessible screens recursively | ||
| 2294 | def processedScreens = [] as Set<String> | ||
| 2295 | ec.logger.info("list#Tools: Starting recursive processing from ${allScreens.size()} base screens") | ||
| 2296 | for (screenPath in allScreens) { | ||
| 2297 | processScreenWithSubscreens(screenPath, null, processedScreens, tools, null, 0) | ||
| 2298 | } | ||
| 2299 | ec.logger.info("list#Tools: Recursive processing found ${tools.size()} total tools") | ||
| 2300 | |||
| 2301 | } finally { | ||
| 2302 | if (adminUserInfo != null) { | ||
| 2303 | ec.user.popUser() | ||
| 2304 | } | ||
| 2305 | } | ||
| 2306 | |||
| 2307 | // Pagination (same as original) | ||
| 2308 | def pageSize = 50 | ||
| 2309 | def startIndex = cursor ? Integer.parseInt(cursor) : 0 | ||
| 2310 | def endIndex = Math.min(startIndex + pageSize, tools.size()) | ||
| 2311 | def paginatedTools = tools.subList(startIndex, endIndex) | ||
| 2312 | |||
| 2313 | result = [tools: paginatedTools] | ||
| 2314 | if (endIndex < tools.size()) { | ||
| 2315 | result.nextCursor = String.valueOf(endIndex) | ||
| 2316 | } | ||
| 2317 | |||
| 2318 | ec.logger.info("list#Tools: Found ${tools.size()} tools for user ${originalUsername}") | ||
| 2319 | ]]></script> | ||
| 2320 | </actions> | ||
| 2321 | </service> | ||
| 2322 | |||
| 2025 | <!-- NOTE: handle#McpRequest service removed - functionality moved to screen/webapp.xml for unified handling --> | 2323 | <!-- NOTE: handle#McpRequest service removed - functionality moved to screen/webapp.xml for unified handling --> |
| 2026 | 2324 | ||
| 2027 | </services> | 2325 | </services> | ... | ... |
| ... | @@ -890,7 +890,7 @@ try { | ... | @@ -890,7 +890,7 @@ try { |
| 890 | case "tools/list": | 890 | case "tools/list": |
| 891 | // Ensure sessionId is available to service for notification consistency | 891 | // Ensure sessionId is available to service for notification consistency |
| 892 | if (sessionId) params.sessionId = sessionId | 892 | if (sessionId) params.sessionId = sessionId |
| 893 | return callMcpService("mcp#ToolsList", params, ec) | 893 | return callMcpService("list#Tools", params, ec) |
| 894 | case "tools/call": | 894 | case "tools/call": |
| 895 | // Ensure sessionId is available to service for notification consistency | 895 | // Ensure sessionId is available to service for notification consistency |
| 896 | if (sessionId) params.sessionId = sessionId | 896 | if (sessionId) params.sessionId = sessionId | ... | ... |
-
Please register or sign in to post a comment