CAMEL-24832: camel-openai, camel-spring-ai-chat - propagate the caller's context into ai-tool route invocations - #27264

Solid change — unifies exchange propagation for AI tool invocations across all three runtimes (langchain4j-agent, openai, spring-ai-chat) behind a single AiToolExecutor.createToolExchange() helper, preventing them from drifting apart again.

Verified:

• Exchange isolation: ExchangeHelper.createCopy() (which delegates to AbstractExchange copy constructor) creates a proper deep copy — properties and variables are carried, message/body/exception are isolated. The test confirms this.
• Thread safety for parallel tool calls (openai executeParallel): exchange.copy() only reads from the parent exchange, which is idle during tool execution. Concurrent copies are safe.
• releaseExchange() removal: correct — createCopy() produces a standalone exchange, not a pooled consumer exchange. Matches langchain4j-agent, which never released copies.
• Spring AI closure capture: the callingExchange captured in toToolCallback's lambda is copied fresh on each invocation — no shared mutable state.

No issues.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Thanks for picking this up, @oscerd. Making the route-tool runtimes behave the same through one shared createToolExchange helper is the right shape, and dropping releaseExchange() is correct since the copy is not a pooled consumer exchange. A few things need attention before this can go in.

Blocking

1. The PR does not compile. AiToolExecutor.java uses ExchangeHelper but lacks import org.apache.camel.support.ExchangeHelper; (see the inline comment). Built at cd94b25739e7:

AiToolExecutor.java:[191,16] cannot find symbol: variable ExchangeHelper

2. Tool routes now receive the caller's body and headers too, not only properties and variables. The copy is isolated in one direction (changes do not leak back to the caller), but it starts with the caller's whole message. A probe against camel-openai (with the import added locally): the caller sent body silent, headers CamelHttpUri=/inbound/path and Authorization: Bearer caller-token, and property subject=alice:

PROBE[silent]  -> silent                                   (tool route only logs)
PROBE[headers] -> uri=/inbound/path auth=Bearer caller-token
PROBE[props]   -> subject=alice

• A tool route that never sets a body now returns the user's prompt to the model as the tool result (it was No result before).
• Caller headers reach the tool route, including an inbound HTTP request's Authorization and CamelHttpUri/CamelHttpPath/CamelHttpQuery. A tool route that calls an HTTP endpoint can pick those up and go to the wrong URI or forward credentials.
• A caller header whose name matches an optional tool argument shows up as that argument when the model leaves it out.

langchain4j-agent has behaved this way since CAMEL-23944, but that fix was about side effects leaking back. For openai and spring-ai this is new. Suggestion: have createToolExchange clear the copied message's headers and body, keeping properties and variables. That matches the new doc section and is all the property-based authorization pattern needs. It also changes langchain4j-agent, so it belongs in the upgrade guide. Either way the doc should say exactly what reaches the tool route.

Also needed

3. Upgrade guide. On openai and spring-ai, tool routes now inherit the caller's properties and variables (plus headers and body unless 2 changes that) and share its exchange id. That is a behaviour change for existing users, so it needs an entry in camel-4x-upgrade-guide-4_23.adoc, per the Code Quality rules in CLAUDE.md.

4. The fix itself is not tested. createToolExchangeCopiesCallerContextAndIsolatesHeadersAndBody only tests the helper. If the old fresh-exchange code came back in McpToolCallExecutor or AiToolSpecToSpringAi, every test would still pass. A route-tool test for each runtime that reads ${exchangeProperty.subject} would cover it. In camel-openai that is about 20 lines, following OpenAIRouteToolReturnDirectTest.

5. Stale javadoc. AiToolExecutor.execute() (line 64) still says the caller must create the exchange "and release it afterwards (via consumer.releaseExchange()) in a try-finally block". No caller does that any more.

Questions

• createCopy(exchange, true) keeps the caller's exchange id, so with parallel tool execution every tool exchange in a batch shares it. Is that intended for tracing and correlation?
• Minor: the javadoc of McpToolCallExecutor.execute has no @param for the new callingExchange.

With the import added locally, the existing tests of camel-ai-tool (119), camel-openai (352), camel-spring-ai-chat (13) and camel-langchain4j-agent (74) all pass, so nothing else breaks.

This is a rules-and-conventions review and does not replace specialized review tools or static analysis.

Claude Code on behalf of davsclaus

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Re-review after f5c74b3. Checking the five findings from the previous davsclaus review:

1. Missing ExchangeHelper import → ✅ Addressed.
2. Body/headers leaking into tool routes → ✅ Addressed. createToolExchange() now clears body and headers after copy. The doc section accurately describes what reaches the tool route.
3. Missing upgrade guide entry → ✅ Addressed. Covers all three runtimes.
4. Missing route-tool wiring test → The helper unit test (createToolExchangeCopiesCallerContextButGivesACleanMessage) is good but davsclaus asked for a route-tool test per runtime that reads ${exchangeProperty.subject}. Only the helper is tested.
5. Stale javadoc on AiToolExecutor.execute() → ❌ Still present. Line 66 says: "The calling adapter owns the exchange lifecycle: it must create the exchange before calling this method and release it afterwards (via consumer.releaseExchange()) in a try-finally block." — no caller does that anymore, they all use createToolExchange() which produces a non-pooled copy.

The code change itself is solid — the centralization in createToolExchange() with explicit body/header clearing is the right design.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Re-review after cd63eb1 (squashed rewrite). Checking the five findings from the davsclaus CHANGES_REQUESTED review and the two from the previous gnodet-bot COMMENT review:

1. Missing ExchangeHelper import → ✅ Addressed.
2. Body/headers leaking into tool routes → ✅ Addressed. createToolExchange() copies then clears body and headers.
3. Missing upgrade guide entry → ✅ Addressed. Covers all three runtimes and the langchain4j behaviour change.
4. Missing route-tool wiring test → ✅ Addressed. McpToolCallExecutorTest.routeToolSeesCallerExchangePropertyAndGetsACleanMessage drives the openai wiring end-to-end.
5. Stale javadoc on AiToolExecutor.execute() → ❌ Still present. Line 64 says: "The calling adapter owns the exchange lifecycle: it must create the exchange before calling this method and release it afterwards (via consumer.releaseExchange()) in a try-finally block." No caller does releaseExchange() anymore — they all use createToolExchange(). Should say the caller obtains the exchange via createToolExchange and the copy is not pooled, so it does not need to be released.
6. Missing @param callingExchange on McpToolCallExecutor.execute() → ❌ Still missing (see inline).

The code change itself is solid and well-tested. Two low-severity javadoc nits remain from the previous round.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Re-review after 5a5b4f9 (squash rewrite). Checking the outstanding findings from previous reviews:

1. Missing ExchangeHelper import → ✅ Addressed.
2. Body/headers leaking into tool routes → ✅ Addressed.
3. Missing upgrade guide entry → ✅ Addressed.
4. Missing route-tool wiring test → ✅ Addressed (McpToolCallExecutorTest.routeToolSeesCallerExchangePropertyAndGetsACleanMessage).
5. Stale javadoc on AiToolExecutor.execute() → ❌ Still present. Line 64 says: "The calling adapter owns the exchange lifecycle: it must create the exchange before calling this method and release it afterwards (via consumer.releaseExchange()) in a try-finally block." No caller does releaseExchange() anymore — they all use createToolExchange() which produces a non-pooled copy.
6. Missing @param callingExchange on McpToolCallExecutor.execute() → ❌ Still missing (see inline).

The code change itself is solid and well-tested. Two low-severity javadoc nits remain from the previous rounds.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Thanks @oscerd, the blocking points are fixed: the compile error, the tool exchange now starts with a clean message (properties and variables only) in all three runtimes, and the upgrade guide covers them. A few small things before I approve:

• AiToolExecutor.execute() javadoc still says the calling adapter must create the exchange and release it via consumer.releaseExchange() in a try-finally. No caller does that any more, so please update it.
• McpToolCallExecutor.execute is still missing @param callingExchange (gnodet-bot raised it twice).
• My earlier question on the exchange id: createCopy(..., true) keeps the caller's id, so every parallel tool exchange in a batch has the same id. Is that intended? A sentence in the docs either way would help.

Optional:

• camel-spring-ai-chat has no wiring test. If AiToolSpecToSpringAi.toToolCallback went back to a fresh exchange, nothing would fail. Same for langchain4j-agent.
• The copy also shares the caller's UnitOfWork (and internal properties) across parallel tool calls. That's how langchain4j-agent worked already, but it's new for openai and spring-ai; the "The calling exchange" doc section could mention it.

Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.