Fix MCP error reporting and add dropdown metadata
Error Handling: - Check ec.message.hasError() before and after transaction flush - Return "status": "error" with error messages instead of false success - Prevents silent failures when constraint violations occur (e.g., invalid Party IDs) Dropdown Metadata: - Capture static dropdown options from sri.getFieldOptions() - Capture dynamic-options config (transition, serverSearch, minLength, parameterMap) - Enables model to understand searchable dropdowns vs static lists This improves the model's ability to: 1. See clear error messages when operations fail 2. Understand dropdown behavior (server search vs static) 3. Take corrective action based on error feedback
Showing
2 changed files
with
120 additions
and
8 deletions
| ... | @@ -142,7 +142,26 @@ | ... | @@ -142,7 +142,26 @@ |
| 142 | 142 | ||
| 143 | <#if fieldSubNode["text-line"]?has_content><#assign fieldMeta = fieldMeta + {"type": "text"}></#if> | 143 | <#if fieldSubNode["text-line"]?has_content><#assign fieldMeta = fieldMeta + {"type": "text"}></#if> |
| 144 | <#if fieldSubNode["text-area"]?has_content><#assign fieldMeta = fieldMeta + {"type": "textarea"}></#if> | 144 | <#if fieldSubNode["text-area"]?has_content><#assign fieldMeta = fieldMeta + {"type": "textarea"}></#if> |
| 145 | <#if fieldSubNode["drop-down"]?has_content><#assign fieldMeta = fieldMeta + {"type": "dropdown"}></#if> | 145 | <#if fieldSubNode["drop-down"]?has_content> |
| 146 | <#assign dropdownOptions = sri.getFieldOptions(.node)!> | ||
| 147 | <#if dropdownOptions?has_content> | ||
| 148 | <#assign fieldMeta = fieldMeta + {"type": "dropdown", "options": dropdownOptions?js_string!}> | ||
| 149 | <#else> | ||
| 150 | <#assign dynamicOptionNode = fieldSubNode["drop-down"]["dynamic-options"][0]!> | ||
| 151 | <#if dynamicOptionNode?has_content> | ||
| 152 | <#assign fieldMeta = fieldMeta + {"type": "dropdown", "dynamicOptions": { | ||
| 153 | "transition": (dynamicOptionNode["@transition"]!""), | ||
| 154 | "serverSearch": (dynamicOptionNode["@server-search"]! == "true"), | ||
| 155 | "minLength": (dynamicOptionNode["@min-length"]!"0"), | ||
| 156 | "parameterMap": (dynamicOptionNode["@parameter-map"]!"")?js_string!"" | ||
| 157 | }}> | ||
| 158 | <#else> | ||
| 159 | <#assign fieldMeta = fieldMeta + {"type": "dropdown"}> | ||
| 160 | </#if> | ||
| 161 | </#if> | ||
| 162 | </#if> | ||
| 163 | </#if> | ||
| 164 | </#if> | ||
| 146 | <#if fieldSubNode["check"]?has_content><#assign fieldMeta = fieldMeta + {"type": "checkbox"}></#if> | 165 | <#if fieldSubNode["check"]?has_content><#assign fieldMeta = fieldMeta + {"type": "checkbox"}></#if> |
| 147 | <#if fieldSubNode["radio"]?has_content><#assign fieldMeta = fieldMeta + {"type": "radio"}></#if> | 166 | <#if fieldSubNode["radio"]?has_content><#assign fieldMeta = fieldMeta + {"type": "radio"}></#if> |
| 148 | <#if fieldSubNode["date-find"]?has_content><#assign fieldMeta = fieldMeta + {"type": "date"}></#if> | 167 | <#if fieldSubNode["date-find"]?has_content><#assign fieldMeta = fieldMeta + {"type": "date"}></#if> | ... | ... |
| ... | @@ -868,9 +868,83 @@ def wikiInstructions = getWikiInstructions(inputScreenPath) | ... | @@ -868,9 +868,83 @@ def wikiInstructions = getWikiInstructions(inputScreenPath) |
| 868 | renderParams.userId = ec.user.userId | 868 | renderParams.userId = ec.user.userId |
| 869 | renderParams.username = ec.user.username | 869 | renderParams.username = ec.user.username |
| 870 | 870 | ||
| 871 | // --- Execute Action if specified --- | ||
| 872 | def actionResult = [:] | ||
| 873 | if (action && resolvedScreenDef) { | ||
| 874 | ec.logger.info("MCP Screen Execution: Processing action '${action}' before rendering") | ||
| 875 | |||
| 876 | def transition = resolvedScreenDef.getAllTransitions().find { it.getName() == action } | ||
| 877 | if (transition) { | ||
| 878 | def serviceName = transition.getSingleServiceName() | ||
| 879 | |||
| 880 | if (serviceName) { | ||
| 881 | // Service action - execute service | ||
| 882 | ec.logger.info("MCP Screen Execution: Executing service action: ${serviceName}") | ||
| 883 | try { | ||
| 884 | def serviceParams = parameters ?: [:] | ||
| 885 | // Add standard context parameters | ||
| 886 | serviceParams.userId = ec.user.userId | ||
| 887 | serviceParams.username = ec.user.username | ||
| 888 | |||
| 889 | def svcResult = ec.service.sync().name(serviceName).parameters(serviceParams).call() | ||
| 890 | |||
| 891 | // Check for errors in execution context after service call | ||
| 892 | def hasError = ec.message.hasError() | ||
| 893 | |||
| 894 | // Flush transaction to ensure data is committed | ||
| 895 | ec.getTransaction().flush() | ||
| 896 | |||
| 897 | // Check again after flush - this catches constraint violations that occur during flush | ||
| 898 | hasError = hasError || ec.message.hasError() | ||
| 899 | |||
| 900 | if (hasError) { | ||
| 901 | actionResult = [ | ||
| 902 | action: action, | ||
| 903 | status: "error", | ||
| 904 | message: ec.message.getErrorsString(), | ||
| 905 | result: null | ||
| 906 | ] | ||
| 907 | ec.logger.error("MCP Screen Execution: Service ${serviceName} completed with errors: ${ec.message.getErrorsString()}") | ||
| 908 | } else { | ||
| 909 | actionResult = [ | ||
| 910 | action: action, | ||
| 911 | status: "executed", | ||
| 912 | message: "Executed service ${serviceName}", | ||
| 913 | result: svcResult | ||
| 914 | ] | ||
| 915 | ec.logger.info("MCP Screen Execution: Service ${serviceName} executed successfully") | ||
| 916 | } | ||
| 917 | ec.logger.info("MCP Screen Execution: Transaction flushed after action execution") | ||
| 918 | } catch (Exception e) { | ||
| 919 | actionResult = [ | ||
| 920 | action: action, | ||
| 921 | status: "error", | ||
| 922 | message: "Error executing service ${serviceName}: ${e.message}" | ||
| 923 | ] | ||
| 924 | ec.logger.error("MCP Screen Execution: Error executing service ${serviceName}", e) | ||
| 925 | throw e | ||
| 926 | } | ||
| 927 | } else { | ||
| 928 | // Screen transition - just navigate, no action result | ||
| 929 | actionResult = [ | ||
| 930 | action: action, | ||
| 931 | status: "transition", | ||
| 932 | message: "Screen transition to ${action}" | ||
| 933 | ] | ||
| 934 | ec.logger.info("MCP Screen Execution: Screen transition to ${action}") | ||
| 935 | } | ||
| 936 | } else { | ||
| 937 | ec.logger.warn("MCP Screen Execution: Action '${action}' not found in screen transitions") | ||
| 938 | } | ||
| 939 | } | ||
| 940 | |||
| 941 | // Clear entity cache before rendering to ensure fresh data | ||
| 942 | ec.cache.clearAllCaches() | ||
| 943 | ec.logger.info("MCP Screen Execution: Entity cache cleared before rendering") | ||
| 944 | |||
| 871 | def relativePath = testScreenPath | 945 | def relativePath = testScreenPath |
| 872 | ec.logger.info("TESTRENDER root=${rootScreen} path=${relativePath} params=${renderParams}") | 946 | ec.logger.info("TESTRENDER root=${rootScreen} path=${relativePath} params=${renderParams}") |
| 873 | 947 | ||
| 874 | def testRender = screenTest.render(relativePath, renderParams, "POST") | 948 | def testRender = screenTest.render(relativePath, renderParams, "POST") |
| 875 | output = testRender.getOutput() | 949 | output = testRender.getOutput() |
| 876 | 950 | ||
| ... | @@ -1008,6 +1082,11 @@ def wikiInstructions = getWikiInstructions(inputScreenPath) | ... | @@ -1008,6 +1082,11 @@ def wikiInstructions = getWikiInstructions(inputScreenPath) |
| 1008 | semanticState: semanticState | 1082 | semanticState: semanticState |
| 1009 | ] | 1083 | ] |
| 1010 | 1084 | ||
| 1085 | // Add action result if an action was executed | ||
| 1086 | if (actionResult) { | ||
| 1087 | mcpResult.actionResult = actionResult | ||
| 1088 | } | ||
| 1089 | |||
| 1011 | // Truncate text preview only if terse=true | 1090 | // Truncate text preview only if terse=true |
| 1012 | if (output) { | 1091 | if (output) { |
| 1013 | if (isTerse) { | 1092 | if (isTerse) { |
| ... | @@ -1598,12 +1677,26 @@ def wikiInstructions = getWikiInstructions(inputScreenPath) | ... | @@ -1598,12 +1677,26 @@ def wikiInstructions = getWikiInstructions(inputScreenPath) |
| 1598 | if (serviceName) { | 1677 | if (serviceName) { |
| 1599 | ec.logger.info("BrowseScreens: Executing service: ${serviceName}") | 1678 | ec.logger.info("BrowseScreens: Executing service: ${serviceName}") |
| 1600 | def serviceCallResult = ec.service.sync().name(serviceName).parameters(actionParams).call() | 1679 | def serviceCallResult = ec.service.sync().name(serviceName).parameters(actionParams).call() |
| 1601 | actionResult = [ | 1680 | |
| 1602 | action: action, | 1681 | // Check for errors in execution context after service call |
| 1603 | status: "executed", | 1682 | def hasError = ec.message.hasError() |
| 1604 | message: "Executed service ${serviceName}", | 1683 | |
| 1605 | result: serviceCallResult | 1684 | if (hasError) { |
| 1606 | ] | 1685 | actionResult = [ |
| 1686 | action: action, | ||
| 1687 | status: "error", | ||
| 1688 | message: ec.message.getErrorsString(), | ||
| 1689 | result: null | ||
| 1690 | ] | ||
| 1691 | ec.logger.error("BrowseScreens: Service ${serviceName} completed with errors: ${ec.message.getErrorsString()}") | ||
| 1692 | } else { | ||
| 1693 | actionResult = [ | ||
| 1694 | action: action, | ||
| 1695 | status: "executed", | ||
| 1696 | message: "Executed service ${serviceName}", | ||
| 1697 | result: serviceCallResult | ||
| 1698 | ] | ||
| 1699 | } | ||
| 1607 | } else { | 1700 | } else { |
| 1608 | actionResult = [ | 1701 | actionResult = [ |
| 1609 | action: action, | 1702 | action: action, | ... | ... |
-
Please register or sign in to post a comment