oscerd commented on code in PR #27264:
URL: https://github.com/apache/camel/pull/27264#discussion_r4164488214
##########
components/camel-ai/camel-ai-tool/src/main/java/org/apache/camel/component/ai/tool/AiToolExecutor.java:
##########
@@ -175,4 +175,19 @@ 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, as an isolated copy of
the calling (agent) exchange. Context set
+ * before the agent ran - most importantly the authenticated caller's
identity kept as an exchange property -
+ * reaches the tool route, while header, body and exception changes on the
tool exchange 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 to pass to {@link
#execute(AiToolSpec, Map, Exchange)}
+ */
+ public static Exchange createToolExchange(Exchange callingExchange) {
+ return ExchangeHelper.createCopy(callingExchange, true);
Review Comment:
Both fixed.
The `import org.apache.camel.support.ExchangeHelper;` is restored — that one
was my mistake: a revert-to-red mutation left the import unused, impsort
stripped it, and I pushed without rebuilding. The module compiles again
(verified with a full `mvn clean install -DskipITs` on the four AI modules).
On the second point: `createToolExchange` no longer copies the caller's
whole message. It copies only the caller's exchange **properties and
variables**, then clears the message (body `null`, inbound headers cleared), so
the tool route starts from its own arguments only — and a tool that sets no
body returns `No result` again, not the caller's prompt. Pinned by
`AiToolExecutorTest.createToolExchangeCopiesCallerContextButGivesACleanMessage`;
revert-to-red verified (reverting to a plain full copy turns the
body/inbound-header assertions red while the context assertions stay green).
_Claude Code on behalf of oscerd_
##########
components/camel-ai/camel-ai-tool/src/main/docs/ai-tool-component.adoc:
##########
@@ -353,6 +353,23 @@ YAML::
----
====
+== The calling exchange
+
+Each tool invocation runs on an isolated copy of the *calling exchange* — the
exchange that drives the agent
+(the `langchain4j-agent`, `openai` or `spring-ai-chat` producer). The context
the caller set before the agent ran
+is carried onto the tool route:
+
+* exchange *properties* — most importantly an authenticated caller's identity
kept as a property, so a tool route
+ can be guarded on it (for example `exchangeProperty.subject`) and the model
cannot forge it;
+* exchange *variables*.
+
+The tool's own *headers*, *body* and any *exception* are isolated to the copy
and do not leak back into the calling
+exchange. The tool arguments supplied by the model are placed on this copy
before the route runs.
Review Comment:
Fixed — the doc now matches the behaviour. The helper clears the copied
message, and the "The calling exchange" section now says explicitly that the
tool route receives **only its own arguments** (set as headers), not the
caller's body or inbound headers, and that a tool which sets no body returns
`No result`. Only the caller's exchange **properties and variables** carry
over, so a route can still be guarded on `exchangeProperty.subject`. The
catalog mirror is regenerated to match.
_Claude Code on behalf of oscerd_
##########
components/camel-ai/camel-openai/src/main/java/org/apache/camel/component/openai/McpToolCallExecutor.java:
##########
@@ -279,30 +282,29 @@ private ToolResult executeRouteTool(
try {
Map<String, Object> argsMap = OBJECT_MAPPER.readValue(argsJson,
Map.class);
- Exchange toolExchange = spec.getConsumer().createExchange(false);
- try {
- AiToolResult result = AiToolExecutor.execute(spec, argsMap,
toolExchange);
- if (result instanceof AiToolResult.Success success) {
- LOG.debug("Route tool '{}' result: {}", toolName,
success.value());
- return new ToolResult(
- toolCall.asFunction().id(), toolName,
success.value(),
- toolState.returnDirectTools().contains(toolName),
0, true);
- } else if (result instanceof AiToolResult.ArgumentError error)
{
- LOG.warn("Route tool '{}' argument error: {}", toolName,
error.message());
- return errorResult(toolCall, "Error: invalid tool
arguments: " + error.message());
- } else {
- AiToolResult.ExecutionError error =
(AiToolResult.ExecutionError) result;
- if (config.getToolExecutionErrorStrategy() ==
ToolExecutionErrorStrategy.FAIL_EXCHANGE) {
- if (error.cause() != null) {
- throw error.cause();
- }
- throw new IllegalStateException(error.message());
+ // isolated copy of the calling exchange so the caller's context
(e.g. an authenticated subject kept as an
+ // exchange property) reaches the tool route; this is not a pooled
consumer exchange, so it is not released
+ // here (CAMEL-24832)
+ Exchange toolExchange =
AiToolExecutor.createToolExchange(callingExchange);
Review Comment:
Addressed on both counts.
**Behaviour:** the tool exchange no longer starts from the caller's message.
`createToolExchange` copies the caller's properties and variables, then clears
the message — so a route tool that never sets a body returns `No result` (not
the user prompt) and does not see the caller's inbound headers.
**Test:** added
`McpToolCallExecutorTest.routeToolSeesCallerExchangePropertyAndGetsACleanMessage`,
which exercises this exact openai wiring: it registers an `ai-tool:` route,
drives `executor.execute(...)` with a calling exchange carrying
`CamelAuthenticatedSubject=alice`, and asserts (a) the tool route reads that
exchange property (= `alice`), so the **calling** exchange is copied in —
regressing to a fresh exchange turns this red — and (b) a no-body tool returns
`No result`. The shared-helper contract is additionally pinned by
`AiToolExecutorTest.createToolExchangeCopiesCallerContextButGivesACleanMessage`
(revert-to-red verified).
_Claude Code on behalf of oscerd_
--
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]
