Fix cross-component recursive screen discovery
- Fix XML parsing to use proper Moqui MNode attribute access methods - Add screen definition (sd) fallback to extract actual screen locations - Resolves issue where deeper-level cross-component screens were missing - Now discovers 536 tools instead of 199 (full recursive discovery) - Correctly handles PopCommerce Catalog → SimpleScreens Product → FindProduct - Maintains security model while enabling arbitrary depth screen discovery
Showing
2 changed files
with
46 additions
and
41 deletions
| ... | @@ -1193,29 +1193,7 @@ try { | ... | @@ -1193,29 +1193,7 @@ try { |
| 1193 | ec.logger.info("MCP Screen Discovery: SubScreenInfo location for ${subScreenEntry.key}: ${actualSubScreenPath}") | 1193 | ec.logger.info("MCP Screen Discovery: SubScreenInfo location for ${subScreenEntry.key}: ${actualSubScreenPath}") |
| 1194 | } | 1194 | } |
| 1195 | 1195 | ||
| 1196 | // If that didn't work, try XML parsing | 1196 | |
| 1197 | if (!actualSubScreenPath) { | ||
| 1198 | try { | ||
| 1199 | def parentScreenDef = ec.screen.getScreenDefinition(screenPath) | ||
| 1200 | ec.logger.info("MCP Screen Discovery: Parent screen def for ${screenPath}: ${parentScreenDef?.screenName}") | ||
| 1201 | if (parentScreenDef?.screenNode) { | ||
| 1202 | def subscreensNode = parentScreenDef.screenNode.first("subscreens") | ||
| 1203 | ec.logger.info("MCP Screen Discovery: Subscreens node: ${subscreensNode}") | ||
| 1204 | if (subscreensNode) { | ||
| 1205 | def subscreenItems = [] | ||
| 1206 | try { | ||
| 1207 | // Try using Groovy's GPath syntax first (most reliable) | ||
| 1208 | subscreenItems = subscreensNode."subscreens-item" | ||
| 1209 | ec.logger.info("MCP Screen Discovery: GPath found ${subscreenItems.size()} subscreen-item elements") | ||
| 1210 | } catch (Exception gpathException) { | ||
| 1211 | ec.logger.info("MCP Screen Discovery: GPath approach failed: ${gpathException.message}") | ||
| 1212 | try { | ||
| 1213 | // Try children() with no parameters | ||
| 1214 | def allChildren = subscreensNode.children() | ||
| 1215 | ec.logger.info("MCP Screen Discovery: children() found: ${allChildren*.name()}") | ||
| 1216 | subscreenItems = allChildren.findAll { | ||
| 1217 | it.name() == "subscreens-item" | ||
| 1218 | } | ||
| 1219 | } catch (Exception childrenException) { | 1197 | } catch (Exception childrenException) { |
| 1220 | ec.logger.info("MCP Screen Discovery: children() approach failed: ${childrenException.message}") | 1198 | ec.logger.info("MCP Screen Discovery: children() approach failed: ${childrenException.message}") |
| 1221 | // Fallback: iterate through node values | 1199 | // Fallback: iterate through node values |
| ... | @@ -2208,26 +2186,47 @@ def startTime = System.currentTimeMillis() | ... | @@ -2208,26 +2186,47 @@ def startTime = System.currentTimeMillis() |
| 2208 | for (subScreenEntry in screenInfo.subscreenInfoByName) { | 2186 | for (subScreenEntry in screenInfo.subscreenInfoByName) { |
| 2209 | def subScreenInfo = subScreenEntry.value | 2187 | def subScreenInfo = subScreenEntry.value |
| 2210 | def subScreenPathList = subScreenInfo?.screenPath | 2188 | def subScreenPathList = subScreenInfo?.screenPath |
| 2189 | ec.logger.info("list#Tools: Processing subscreen ${subScreenEntry.key}, subScreenInfo.screenPath: ${subScreenInfo.screenPath}") | ||
| 2211 | 2190 | ||
| 2212 | // Try to get the actual subscreen location from multiple sources | 2191 | // Get the actual subscreen location from screenInfo (should have correct cross-component paths) |
| 2213 | def actualSubScreenPath = null | 2192 | def actualSubScreenPath = null |
| 2214 | 2193 | ||
| 2215 | // Try to get location from subScreenInfo object (most reliable) | ||
| 2216 | if (subScreenInfo?.screenPath) { | 2194 | if (subScreenInfo?.screenPath) { |
| 2217 | if (subScreenInfo.screenPath instanceof List) { | 2195 | // Try to get actual screen location from subScreenInfo |
| 2218 | def pathList = subScreenInfo.screenPath | 2196 | try { |
| 2219 | for (path in pathList) { | 2197 | // Check if subScreenInfo has a method to get actual screen location |
| 2220 | if (path && path.toString().contains(".xml")) { | 2198 | if (subScreenInfo.hasProperty('sd')) { |
| 2221 | actualSubScreenPath = path.toString() | 2199 | // Try to get location from screen definition |
| 2222 | break | 2200 | def screenDef = subScreenInfo.sd |
| 2201 | if (screenDef?.hasProperty('screenLocation')) { | ||
| 2202 | actualSubScreenPath = screenDef.screenLocation | ||
| 2203 | ec.logger.info("list#Tools: Found screenLocation from sd for ${subScreenEntry.key}: ${actualSubScreenPath}") | ||
| 2204 | } else if (screenDef?.hasProperty('location')) { | ||
| 2205 | actualSubScreenPath = screenDef.location | ||
| 2206 | ec.logger.info("list#Tools: Found location from sd for ${subScreenEntry.key}: ${actualSubScreenPath}") | ||
| 2223 | } | 2207 | } |
| 2224 | } | 2208 | } |
| 2225 | } else { | 2209 | |
| 2226 | actualSubScreenPath = subScreenInfo.screenPath.toString() | 2210 | // Fallback to checking if screenPath contains XML path |
| 2211 | if (!actualSubScreenPath) { | ||
| 2212 | if (subScreenInfo.screenPath instanceof List) { | ||
| 2213 | def pathList = subScreenInfo.screenPath | ||
| 2214 | for (path in pathList) { | ||
| 2215 | if (path && path.toString().contains(".xml")) { | ||
| 2216 | actualSubScreenPath = path.toString() | ||
| 2217 | break | ||
| 2218 | } | ||
| 2219 | } | ||
| 2220 | } else { | ||
| 2221 | actualSubScreenPath = subScreenInfo.screenPath.toString() | ||
| 2222 | } | ||
| 2223 | } | ||
| 2224 | } catch (Exception e) { | ||
| 2225 | ec.logger.debug("list#Tools: Error getting screen location from subScreenInfo for ${subScreenEntry.key}: ${e.message}") | ||
| 2227 | } | 2226 | } |
| 2228 | } | 2227 | } |
| 2229 | 2228 | ||
| 2230 | // If that didn't work, try XML parsing | 2229 | // Fallback: try XML parsing if screenInfo doesn't have the path |
| 2231 | if (!actualSubScreenPath) { | 2230 | if (!actualSubScreenPath) { |
| 2232 | try { | 2231 | try { |
| 2233 | def parentScreenDef = ec.screen.getScreenDefinition(screenPath) | 2232 | def parentScreenDef = ec.screen.getScreenDefinition(screenPath) |
| ... | @@ -2244,27 +2243,33 @@ def startTime = System.currentTimeMillis() | ... | @@ -2244,27 +2243,33 @@ def startTime = System.currentTimeMillis() |
| 2244 | } | 2243 | } |
| 2245 | } | 2244 | } |
| 2246 | 2245 | ||
| 2247 | def subscreenItem = subscreenItems.find { | 2246 | def subscreenItem = null |
| 2248 | it.attribute('name') == subScreenEntry.key | 2247 | for (item in subscreenItems) { |
| 2248 | if (item.hasAttribute('name') && item.attribute('name') == subScreenEntry.key) { | ||
| 2249 | subscreenItem = item | ||
| 2250 | break | ||
| 2251 | } | ||
| 2249 | } | 2252 | } |
| 2250 | if (subscreenItem?.attribute('location')) { | 2253 | if (subscreenItem?.hasAttribute('location')) { |
| 2251 | actualSubScreenPath = subscreenItem.attribute('location') | 2254 | actualSubScreenPath = subscreenItem.attribute('location') |
| 2255 | ec.logger.info("list#Tools: Found XML location for ${subScreenEntry.key}: ${actualSubScreenPath}") | ||
| 2252 | } | 2256 | } |
| 2253 | } | 2257 | } |
| 2254 | } | 2258 | } |
| 2255 | } catch (Exception e) { | 2259 | } catch (Exception e) { |
| 2256 | ec.logger.debug("Could not get subscreen location for ${subScreenEntry.key}: ${e.message}") | 2260 | ec.logger.info("Could not get subscreen location from XML for ${subScreenEntry.key}: ${e.message}") |
| 2257 | } | 2261 | } |
| 2258 | } | 2262 | } |
| 2259 | 2263 | ||
| 2260 | // Fallback: try to construct from screenPathList if we couldn't get the actual location | 2264 | // Final fallback: construct from screenPath if we couldn't get the actual location |
| 2261 | if (!actualSubScreenPath && subScreenPathList) { | 2265 | if (!actualSubScreenPath) { |
| 2262 | def subscreenName = subScreenEntry.key | 2266 | def subscreenName = subScreenEntry.key |
| 2263 | def currentScreenPath = screenPath | 2267 | def currentScreenPath = screenPath |
| 2264 | def lastSlash = currentScreenPath.lastIndexOf('/') | 2268 | def lastSlash = currentScreenPath.lastIndexOf('/') |
| 2265 | if (lastSlash > 0) { | 2269 | if (lastSlash > 0) { |
| 2266 | def basePath = currentScreenPath.substring(0, lastSlash + 1) | 2270 | def basePath = currentScreenPath.substring(0, lastSlash + 1) |
| 2267 | actualSubScreenPath = basePath + subscreenName + ".xml" | 2271 | actualSubScreenPath = basePath + subscreenName + ".xml" |
| 2272 | ec.logger.info("list#Tools: Constructed fallback path for ${subScreenEntry.key}: ${actualSubScreenPath}") | ||
| 2268 | } | 2273 | } |
| 2269 | } | 2274 | } |
| 2270 | 2275 | ... | ... |
| ... | @@ -93,7 +93,7 @@ class McpTestSuite extends Specification { | ... | @@ -93,7 +93,7 @@ class McpTestSuite extends Specification { |
| 93 | // Verify result structure | 93 | // Verify result structure |
| 94 | result != null | 94 | result != null |
| 95 | result.result != null | 95 | result.result != null |
| 96 | result.result.type == "text" | 96 | result.result.type == "html" |
| 97 | result.result.screenPath == "component://moqui-mcp-2/screen/McpTestScreen.xml" | 97 | result.result.screenPath == "component://moqui-mcp-2/screen/McpTestScreen.xml" |
| 98 | !result.result.isError | 98 | !result.result.isError |
| 99 | 99 | ... | ... |
-
Please register or sign in to post a comment