allthingssecurity commented on code in PR #27264:
URL: https://github.com/apache/camel/pull/27264#discussion_r4171814057
##########
components/camel-ai/camel-ai-tool/src/main/java/org/apache/camel/component/ai/tool/AiToolExecutor.java:
##########
@@ -175,4 +176,28 @@ private static AiToolResult buildSuccessResult(AiToolSpec
spec, Exchange exchang
String.format("Error executing tool '%s': %s",
spec.getName(), e.getMessage()), e);
}
}
+
+ /**
+ * Builds the exchange used to invoke a route tool from the calling
(agent) exchange. The caller's <em>context</em>
+ * is carried over - exchange properties (most importantly the
authenticated caller's identity, so a tool route can
+ * be guarded on {@code exchangeProperty.subject} and the model cannot
forge it) and variables - but the tool route
+ * is given a <em>clean message</em>: it receives only its own tool
arguments (set as headers by
+ * {@link #execute(AiToolSpec, Map, Exchange)}), not the caller's body or
inbound headers, and a tool that sets no
+ * body returns {@code No result} rather than echoing the caller's body
back to the model. Changes the tool makes
+ * are isolated to this copy and do not leak back into the calling
exchange. Every route-tool runtime
+ * (langchain4j-agent, openai, spring-ai-chat) builds the tool exchange
this way, so an authorization check on an
+ * exchange property behaves identically across them (CAMEL-24832,
CAMEL-23944).
+ *
+ * @param callingExchange the exchange driving the agent
+ * @return an isolated copy, carrying the caller's
properties and variables but a clean message, to
+ * pass to {@link #execute(AiToolSpec, Map,
Exchange)}
+ */
+ public static Exchange createToolExchange(Exchange callingExchange) {
+ // copy carries properties and variables; then wipe the message so the
tool route starts from its arguments
+ // only, not the caller's body/headers
+ Exchange toolExchange = ExchangeHelper.createCopy(callingExchange,
true);
+ toolExchange.getMessage().setBody(null);
Review Comment:
One more thing about the copy, which builds on @davsclaus's notes about the
shared exchange id and UnitOfWork. `createCopy(callingExchange, true)` keeps
the caller's UnitOfWork (`AbstractExchange` copies `unitOfWork`) and its
exchange id. So the tool route runs inside the caller's UoW, and that changes
how tool routes behave compared with the fresh exchange openai/spring-ai used
before.
I probed it in camel-ai-tool. A `direct:caller` route calls
`AiToolExecutor.execute` with body `caller-prompt` and an inbound header:
| tool route | fresh exchange (main) | `createToolExchange` (this PR) |
|---|---|---|
| has `onCompletion().process(...)` | runs right after the tool call | never
runs, not even after the caller completes |
| exchange id in 3 parallel calls | 3 distinct ids | all the caller's id |
| `errorHandler(deadLetterChannel("mock:dlq").useOriginalMessage())`, route
throws | DLQ body `null`; tool result `No result` | DLQ gets the caller's body
and headers; tool result `caller-prompt` |
The last row undoes the clean message: `useOriginalMessage()` restores the
UoW's original message, which is the caller's. The DLC handles the exception,
so `execute` returns the restored body to the model.
Giving the copy its own UoW (and its own id) fixes all three rows:
```suggestion
Exchange toolExchange = callingExchange.copy();
// the tool route runs in its own unit of work (onCompletion,
useOriginalMessage, inflight), not in the caller's
toolExchange.getExchangeExtension().setUnitOfWork(null);
toolExchange.getMessage().setBody(null);
```
(and the `ExchangeHelper` import goes away). With that, the camel-ai-tool
tests (120, including
`createToolExchangeCopiesCallerContextButGivesACleanMessage`) and
camel-openai's `McpToolCallExecutorTest` (12, including
`routeToolSeesCallerExchangePropertyAndGetsACleanMessage`) pass. Each tool call
gets its own exchange id, its `onCompletion` runs once, and the DLQ gets the
tool's own original message. That would also settle the exchange-id question:
the id is no longer shared. If you want to keep a link to the caller,
`ExchangePropertyKey.CORRELATION_ID` can carry the caller's id, as
split/multicast do. The "The calling exchange" doc section and the upgrade
guide could then say the tool route runs in its own unit of work.
_Claude Code on behalf of allthingssecurity_
--
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]
