gnodet-bot commented on code in PR #27840:
URL: https://github.com/apache/camel/pull/27840#discussion_r4275696410
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiMcpServer.java:
##########
@@ -532,10 +532,19 @@ private JsonObject handleToolsCall(JsonObject request) {
String text;
boolean isError = false;
+ ToolDescriptor tool = toolName != null ?
ToolRegistry.findTool(toolName) : null;
try {
- JsonObject repeat = toolName != null
- ? repeatedCalls.repeatOf(ToolRegistry.findTool(toolName),
args) : null;
- text = repeat != null ? repeat.toJson() :
toolRegistry.execute(toolName, args);
+ JsonObject repeat = repeatedCalls.repeatOf(tool, args);
+ if (repeat != null) {
+ text = repeat.toJson();
+ } else {
+ text = toolRegistry.execute(toolName, args);
+ // executeShared catches ToolExecutionException and returns
"Error: ..." as normal text; only
+ // record the answer when the call actually succeeded so
failed calls do not use the retry budget
Review Comment:
💡 **Nit (non-blocking):** The `!text.startsWith("Error:")` heuristic works
for the `ToolExecutionException` path (where `executeShared` always returns
`"Error: ..."`), but it would also suppress `recordSuccessfulAnswer` for any
tool whose *valid* response happens to start with `"Error:"` (e.g. a diagnostic
tool summarising an error message). In practice there are none today, but it
couples the success-detection logic to string formatting.
A slightly more robust option — if the refactor scope is acceptable — is to
throw instead of returning an error string from `executeShared`, and catch it
here. Non-blocking: the current approach is still correct for all existing
tools.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
